702046 - Child elements sometimes render outside containing panel div

Status: RESOLVED FIXED

(Socorro :: Webapp, task)

Description

[Lonnen :lonnen]

Attachment: example screenshot

'panel' class div elements will scale smaller, horizontally, than the tables they contain. See attached screenshot. Happens across different reports (individual crash, tcbs, etc).


osx FF 6.0 and up

Comment 1

[Lonnen :lonnen]

Attachment: other example

also happens with canvas, code, and probably pre elements

Comment 2

[Schalk Neethling [:espressive]]

First problem found:

If correlations load successfully there is a class called .correlation-preview that has the value of white-space set to nowrap so, if the correlation has a long signature, for example:

ADD-ON: {22119944-ED35-4ab1-910B-E619EA06A115} (Roboform Toolbar for Firefox, https://addons.mozilla.org/addon/750)

This causes the Correlation columns to stretch outside the container. Removing the white-space setting fixes this.

Comment 3

[Schalk Neethling [:espressive]]

[:lonnen] Pages such as this one: https://crash-stats.mozilla.com/report/index/53eb7906-2d0d-4521-a2d8-4d3b02111112 is always going to be a problem due to the content they contain. Unlike the correlation column, there is no white-space: nowrap rule here nor would it be possible to force these to wrap, as it is a continuous line of 240 characters.

We have a couple of options though.

1) We make the panel div always wrap it's content. Not optimal as it will throw of the alignment of the larger page elements(header, body, footer) and cause horizontal srolling.
2) We have to decide/calculate a optimal number of characters that can be in a cell that contains content of this type and then, always trim down the content to this number of characters and, add the full text to the title tag or a tooltip we can use consistently accross the site.

I would suggest number 2 but, we will have to do the trimming on the server side, either in the PHP or lower down, as some of these tables contain a great number of rows and manipulating the content with JS after DOM ready is going to be expensive, DOM manipulation wise, and cause a jerky rendering experience.

I have not found to many of these as of yet but, I am sure there are more exmaples. Thoughts?

Comment 4

[Lonnen :lonnen]

The second solution is really complicated. Some of our reports (*cough*topcrasher*cough*) take up a lot of horizontal space, but that's really a separate issue. Here we just want to make sure that the panel class stays wide enough to contain its child elements.

Making panel divs `display:inline-block;` is probably enough. I tried it on a handful of pages and it kept the child elements from horizontally without messing too much with the vertical arrangements. See any problems with that approach?

Comment 5

[Schalk Neethling [:espressive]]

(In reply to Chris Lonnen :lonnen from comment #4)
> 
> Making panel divs `display:inline-block;` is probably enough. I tried it on
> a handful of pages and it kept the child elements from horizontally without
> messing too much with the vertical arrangements. See any problems with that
> approach?

No, using float:left would have the same effect. Thing is, if we do that, then the panel DIV will resize based on the width of the content inside it. You can see this happening here https://crash-stats.mozilla.com/report/index/53eb7906-2d0d-4521-a2d8-4d3b02111112, if you set the panel DIV to display: inline-block or set it to float and then click between the different tabs at the top.

We could combine this with a min-width though which would make this much better but, we will still sit with the horizontal scrolling in some places.

I feel though that this is a move in the right direction and we should follow this up with fixing the problem reports such as *cough*topcrasher*cough* ;)

I am going to make this change and set the min-width to 1024, should be fine for the users of the system I believe, and send a pull request.

Comment 6

[Schalk Neethling [:espressive]]

https://github.com/mozilla/socorro/pull/149

Comment 7

[Lonnen :lonnen]

The fix works as advertised, but I'm unclear why you've added a min-width for the panels.

Comment 8

[Schalk Neethling [:espressive]]

[:lonnen] The min-width was added to lessen the impact of the panel scaling wider and narrower depending on the content inside it. If you feel this is unnecessary I can remove it.

Comment 9

[Lonnen :lonnen]

As we discussed in IRC, adding the min-width breaks the few places where we have multiple columns. Also, people look at socorro from phones and tablets, so setting things to a minimum of 1024 wide is a little too much.

To mitigate the reports changing size as you cycle tabs, maybe adding a second class to the reports panels only and giving it a less aggressive min-width would help.

In TheFuture(tm) we can address the larger issue of fixed elements in a fluid column system, but it's too much to tackle right now.

Comment 10

[Schalk Neethling [:espressive]]

Updated pull request https://github.com/mozilla/socorro/pull/150

Comment 11

[Schalk Neethling [:espressive]]

[:lonnen] Added content-wrap to daily reports, here is the commit:
https://github.com/ossreleasefeed/socorro/commit/29a03f84da7bf2f3386434310fe01c9837d2222f

Comment 12

[Lonnen :lonnen]

Attachment: Screencap of problems

Doesn't fix the leftmost daily panel or the branch sources admin panel.

Comment 13

[Schalk Neethling [:espressive]]

[:lonnen] Thanks for the update will handle those cases as well.

Comment 14

[Schalk Neethling [:espressive]]

[:lonnen] I have added the new class to the panel on the branch data sources page. It is currently on the wrapper for the graph as well as the table at the bottom. I did add it to the search panel on the left as well but, this breaks the layout of that page (as can be seen on the screen shot).

I can fix this by floating the graph right but, I have not seen the content stretch outside the container for the left hand search field (i.e. Select a report). I am going to merge the commits and add this as well. Let me know if there are any other places adding this class will be helpful.

Comment 15

[Schalk Neethling [:espressive]]

Attachment: Daily reports with content-wrap on search fields container

Comment 16

[Schalk Neethling [:espressive]]

[:lonnen] Updated pull request :: https://github.com/mozilla/socorro/pull/150

Comment 17

[Lonnen :lonnen]

The page layout is kind of broken with your patch as it was anyway, because of the min-width attribute. The ratio of those two panels seem to be 30% for the left, 60% for the right with a ~10% margin (the actual margin is set to 1 em and not 10%, I don't know why).

Setting the right panel to hold at a minimum width of 800px causes it to bump to a lower line if the window is < ~1300px, which is most of the time for me. If I try and open the dev tools inspector to get the browser width it happens all the time. This will need a different solution.

Since none of the content in the branch panel would stretch it beyond 800px, you've essentially fixed the width at 800px. I'm not all that opposed to that, though. I think that part of the patch will work.

Comment 18

[Schalk Neethling [:espressive]]

[:lonnen] For the daily report page, the top left container does not really present a problem and can have a 'fixed' width. The graph container and table container on the other does need it's container to expand and contract based on the content inside.

I am therefore thinking for this particular page, I will use the normal content-wrap class for the table container, leave the top left container as it was and then set the graph container to display: inline-block but, without the min-width.

I am then also going to remove the content-wrap class from the outer containing panel and see whether this combination works better.

Comment 19

[Schalk Neethling [:espressive]]

Added to a couple more places and updated commit:
https://github.com/mozilla/socorro/pull/150

Comment 22

[Lonnen :lonnen]

Attachment: still hapennin'

This has been marked fixed, but I don't see a pull req, it's not in a milestone, and I can still repro the issue after today's release.

Comment 23

[Lonnen :lonnen]

Whoops. Didn't intend to reassign this to myself.