Closed
Bug 1507736
Opened 6 years ago
Closed 6 years ago
Sidebar misses flex items on webflow.com
Categories
(DevTools :: Inspector, defect, P3)
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.
Reporter | ||
Comment 1•6 years 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.
Comment 2•6 years ago
|
||
.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.
Comment 3•6 years ago
|
||
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.
Updated•6 years ago
|
Priority: -- → P3
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → mtigley
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•6 years ago
|
||
Comment 5•6 years ago
|
||
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•6 years ago
|
||
Thanks! I'll put in a test-case for this in the same patch and have it landed together.
Flags: needinfo?(mtigley)
Assignee | ||
Comment 7•6 years 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•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox65:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
You need to log in
before you can comment on or make changes to this bug.
Description
•