Closed Bug 1507736 Opened 6 years ago Closed 6 years ago

Sidebar misses flex items on webflow.com

Categories

(DevTools :: Inspector, defect, P3)

65 Branch
defect

Tracking

(firefox65 fixed)

RESOLVED FIXED
Firefox 65
Tracking Status
firefox65 --- fixed

People

(Reporter: mbalfanz, Assigned: mtigley)

References

Details

Attachments

(2 files)

STR: - visit webflow.com - open the inspector and select .nav_inner => The element has children, but none show as flex items - next, select the child .nav_main => The item shows as "Flex Item of div.nav_inner" in the sidebar, above the back arrow In this situation it' not clear if .nav_main is actually a flex-item or not.
Also note that in this situation, the space next to the arrow is blank. Instead, it should show the flex-item select with a single item inside.
.nav_main is not a flex item in this situation because it is absolutely positioned in its container. We should probably show a label like "no flex items" where we normally list them, to make the UI less weird. Now, when you select .nav_main there is a bug, we shouldn't be showing the flex item accordion, because this element is not a flex item.
Here's how I think we should fix the issue: 1. In the FlexItemList React component, if flexItems.length = 0 then we should display a message like "No flex items" (this means that we should still call the renderFlexItemList's method of the Flexbox container, when there are 0 items, so we should remove the >0 check from there) 2. On the server, in the LayoutActor's getCurrentDisplay method, we should check that the parent flex container we find when walking up through parents indeed contains the node as a flex item. Today, the problem is that, given a start node, we walk up until we find a non-display:contents parent that is a flex container, but when we find it, we don't check if that container actually considers this start node as a flex item. So if you give it an absolutely positioned element which happens to be a child of a flex container, then it will give you an incorrect result. It should really return null in this case.
Priority: -- → P3
Assignee: nobody → mtigley
Status: NEW → ASSIGNED
Thanks Micah, I just approved the revision. Now we should also add a test to cover this. I know you have already modified one to handle the "No items" case, but ideally we also need one to handle the case where an element that is a child of a container but that is not an item is selected. Test case: data:text/html,<div style="display:flex"><div style="position:absolute;">test</div></div> Test: select the nested <div>, and check that the flex accordion is empty. It's fine to do this in a separate patch if you prefer, since I've already R+'d the first one (in fact, it's even fine to mark this bug as "leave-open" and land the first patch already).
Flags: needinfo?(mtigley)
Thanks! I'll put in a test-case for this in the same patch and have it landed together.
Flags: needinfo?(mtigley)
Pushed by mtigley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ed8f77cd2f9e Make sure the Flexbox inspector correctly shows if the current flex container is also a flex item. r=pbro
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: