Avoid overlapping labels in the flex item outline

RESOLVED FIXED in Firefox 65

Status

enhancement
P2
normal
RESOLVED FIXED
9 months ago
7 months ago

People

(Reporter: pbro, Assigned: pbro)

Tracking

unspecified
Firefox 65
Dependency tree / graph

Firefox Tracking Flags

(firefox65 fixed)

Details

Attachments

(4 attachments)

On treeherder, select `.Pane.horizontal.Pane1`.
The outline is displayed for this flex item, but the basis and final points now cover themselves up.

One idea would be to detect when points have the same value, and in this case just draw 1 of the 2 points only, with the 2 labels.

Here, instead of drawing basis and final independently, and worrying about positioning the labels in a way that they don't overlap, draw just one point with the label "basis\nfinal" so they're for sure not going to overlap.
Assignee: nobody → mtigley
Status: NEW → ASSIGNED
A random thought, maybe it's helpful: if 2 points have the same coordinate (or maybe are close enough, like < 3px apart), then we should draw only one point, and make the label for that point contain both labels.
Basically, we could loop over all points we're thinking of drawing before starting to draw, and accumulate the ones that are very close into just 1 point.
I hope this makes sense. If not, I can try to explain better.
Duplicate of this bug: 1505263

Comment 6

8 months ago
Pushed by mtigley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/39825c67e39c
Avoid overlapping labels in the flex item outline. r=pbro

Comment 8

8 months ago
Backout by shindli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/12c2fb24c691
Backed out changeset 39825c67e39c in  devtools/client/inspector/flexbox/test/browser_flexbox_item_outline_renders_basisfinal_points_correctly.js CLOSED TREE

Comment 9

8 months ago
Pushed by mtigley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/17a21d9e1420
Avoid overlapping labels in the flex item outline. r=pbro

Comment 10

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/17a21d9e1420
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65

Comment 11

8 months ago
Posted image overlap.PNG
Verified this issue on the latest Nightly build (20181112100105) using the following example, and the issue can still be reproduced under certain conditions: 

data:text/html,<div style="display:flex;flex-direction:column"><div style="flex-grow:1;height:305px;max-height:300px"></div><div style="flex-grow:1;height:400px;"></div></div>

Based on this, I'll reopen this issue for further investigation.

Updated

8 months ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Thanks Paul. Indeed a similar problem does persists. We fixed this bug only in cases where final and basis have exactly the same value. But if they differ only slightly (e.g. a few pixels like in your screenshot), then they'll still overlap because we'll attempt to draw them very close to eachother.
I have a fix for this that relies on using min-width in CSS to make sure the items we draw in the outline are always spread-out at least a little bit.
Assignee: mtigley → pbrosset
Status: REOPENED → ASSIGNED
By adding a min-width to the grid items used to display things in the flex outline, we make
sure they each have at least some room to be visible.
Even if that means the outline isn't a perfect 1:1 mapping of what the item actually is.

I also made 2 other minor fixes to the outline:
- using display:grid on the point labels makes sure they fit nicely where they are supposed
  to and the little border drawn next to them to indicate their position doesn't stick out
- I removed a specific override for display the combined final-basis point in column mode
  that was making the border for that point show on the incorrect side

Comment 15

8 months ago
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/42b5ac0ec022
Make sure things displayed in the flex sizing outline have enough room; r=mtigley

Comment 16

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/42b5ac0ec022
Status: ASSIGNED → RESOLVED
Closed: 8 months ago7 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.