Sidebar misses flex items on webflow.com

RESOLVED FIXED in Firefox 65

Status

defect
P3
normal
RESOLVED FIXED
5 months ago
5 months ago

People

(Reporter: mbalfanz, Assigned: mtigley)

Tracking

65 Branch
Firefox 65

Firefox Tracking Flags

(firefox65 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 months ago
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.
(Reporter)

Comment 1

5 months ago
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)

Updated

5 months ago
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)
(Assignee)

Comment 6

5 months ago
Thanks! I'll put in a test-case for this in the same patch and have it landed together.
Flags: needinfo?(mtigley)

Comment 8

5 months ago
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

Comment 9

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ed8f77cd2f9e
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
You need to log in before you can comment on or make changes to this bug.