1496458 - With nested flex containers, can't see flex item properties for inner flex container (unless you scroll past all of its own flex container info)

Status: VERIFIED FIXED

(DevTools :: Inspector, defect, P2)

Description

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

STR:
 1. Visit https://jsfiddle.net/8n6yvu7o/

 2. Inspect the #outer flex container in the flexbox inspector. e.g.:
   - right-click 'aaa' and choose inspect
   - click the flex badge next to it (<div id="outer" class="container">) in the DOM inspector view. 
   - Select the "layout" pane on the right, if it's not already selected.


 3. Click its second item (shown as "2. div#inner.container")

EXPECTED RESULTS: I should see a listing of the flex *item* sizing information for that element. (flex-basis through Final Width)

ACTUAL RESULTS: I end up seeing the flex *container* alignment information for that inner container. (align-content through justify-content, plus a list of its own flex items)

I think I see a brief flash of its item sizing info, but it disappears quickly & is replaced by the container info.

Comment 1

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #0)
> Created attachment 9014445 [details]
> testcase 1
> 
> STR:
>  1. Visit https://jsfiddle.net/8n6yvu7o/

(Sorry -- I forgot I left that jsfiddle in my STR.  That works, but you can also view the more lightweight attached testcase.)

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: screencast of bug

Comment 3

[Daniel Holbert [:dholbert]]

I'm testing using Nightly 64.0a1 (2018-10-04) (64-bit) with a fresh profile, BTW.

Comment 4

[Gabriel Luong [:gl] (ΦωΦ)]

I think this should be fixed.

Comment 5

[Daniel Holbert [:dholbert]]

Nope, I can still reproduce in latest Nightly: 64.0a1 (2018-10-08) (64-bit)

(Though maybe there's a fix in the pipe that didn't quite make it into latest nightly?)

Comment 6

[Patrick Brosset <:pbro>]

So, this works for me as designed, when I select the inner container, I see 2 accordion items: 

- one for showing information about the .inner element properties as a *flex container* and listing its flex items
- one for showing information about the .inner element sizing properties as a *flex item* (further down in the Layout sidebar).

This feature was added as of https://hg.mozilla.org/mozilla-central/rev/1522c7633aa8 which landed 4 days ago I believe.

This is what we discussed should happen for elements that are both items and containers.
Although I seem to remember that we had discussed about moving the item sizing information to be first in the list of accordion items.

Daniel, can you confirm that you also see this feature now?
If so, Gabriel, should we re-purpose this bug to invert the order in which we display this information?

Comment 7

[Daniel Holbert [:dholbert]]

(In reply to Patrick Brosset <:pbro> from comment #6)
> So, this works for me as designed, when I select the inner container, I see
> 2 accordion items:
[...]
> Daniel, can you confirm that you also see this feature now?

Thanks for clarifying. 

On further inspection, I can confirm that the 2nd accordion item for "flex item of div#outercontainer" is there, though I have to scroll the devtools pane before it's visible, so it's effectively not there.

> If so, Gabriel, should we re-purpose this bug to invert the order in which
> we display this information?

Yeah -- one way or another, we should repurpose this bug to ensure the flex item info is reliably visible when clicking the #inner flex item (which happens to also be a container).

Inverting the order would indeed address this, I think, but one other possible solution (which doesn't require reordering stuff): maybe we should autocollapse the "Flex Container" accordion pane for flex containers, if they happen to be a child of the currently-activated flex container?

Comment 8

[Daniel Holbert [:dholbert]]

[setting a user story to make the expected results clearer here]

Comment 9

[Daniel Holbert [:dholbert]]

Attachment: screencast of post-Bug-1478397 behavior (still feels surprising/clunky)

Comment 10

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: 1496458.patch

Comment 11

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: 1496458.patch

Comment 12

[Micah [:mtigley] (she/her)]

Comment on attachment 9016380 [details] [diff] [review]
1496458.patch

Review of attachment 9016380 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good! Verified that accordion order is inverted when the selected node is both a flex container and flex item.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5bb9d00011e
Show the parent flex container accordion at the top if it exists. r=mtigley

Comment 14

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/f5bb9d00011e

Comment 15

[Paul_B]

Attachment: Short video of the issue

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:64.0) Gecko/20100101 Firefox/64.0
Build ID: 20181012103207

Verified this using the latest Nightly build on Windows 10, Mac OS 10.13 and Ubuntu 18.04. 
There are some cases when the sizing details of a nested container do not get displayed (I could not pinpoint the pattern). This issue can be observed in the 1st part of the video I've attached. 

As a result, I've reopened this issue for further investigation. 

The second part of the video is showcasing the same scrolling issue, but this time ''in reverse'', for the flex items of a nested container. Upon selecting a flex item, the user remains in the same scrolling position, having the Box Model displayed, therefore having to scroll all the way up in order to view the sizing details. If he chooses to view the sizing details of another item in that nested container, he has to scroll back down to select it, because when selecting the back arrow, he gets the outer container details displayed.

Comment 16

[Daniel Holbert [:dholbert]]

(In reply to Paul_B from comment #15)
> There are some cases when the sizing details of a nested container do not
> get displayed (I could not pinpoint the pattern). This issue can be observed
> in the 1st part of the video I've attached. 

So in the first part of the video, you don't seem to be inspecting any flex containers that are also flex items.  (The ones you're picking are *grandchildren* of other flex containers -- so the inner ones aren't direct participants in the outer ones' flex layout.)

E.g. your video shows a jump from from #wrapper to .inner, which is skipping over a layer (#header), which is the actual flex item in #wrapper's flex layout & which would show you its flex sizing properties if you were to look at it.

So it's correct that no sizing properties show up when you make that jump. Having said that: there does indeed seem to be a bug here, because we show an empty "flex item" pane with an incorrect title ("flex item of div#wrapper"), when we should simply not showing the flex item pane at all.

(This issue is shown at e.g. 0:07 in your video -- if you just pause at that time, you can see the empty flex-item area, with an element that is **not actually** a flex item being focused.)

> The second part of the video is showcasing the same scrolling issue, but
> this time ''in reverse'', for the flex items of a nested container. Upon
> selecting a flex item, the user remains in the same scrolling position

Yeah, this happens around 0:33-0:35.  The jump from ".links" (a flex container+flex item) to ".link-dashboard" (just a flex item) is broken here. Because the user had to scroll down 2/3 of the way to see .links' flex container properties, they end up with a still-scrolled-down view when they click through to the .link-dashboard flex item, which puts us at a point where none of .link-dashboard's flex-relevant stuff is in view.

Per the user story here, clicking a flex item should "reliably take me directly to the chosen item's Flex Item info pane", and they don't right now.

So I'm not 100% sure that the pane-reversal strategy is really a win here... it happens to help in the case where we're clicking through from an "outer" flex-container to an "inner" flex-item-that-is-also-a-container.  But it seems to have taken us a step away from usability when going from a flex-item-that-is-also-a-container to just-a-plain-flex-item.

Here's a data URL of that scenario:
data:text/html,<div style="display:flex"><div style="display:flex" id="firstOpenMyFlexContainerPane"><div id="andThenClickThroughToThisItem">Hi


In an ideal world, it'd be great to be able to set the scroll position and toggle the collapsed-un-collapsed-ness of panes when the user clicks back and forth between a flex item and its container... but I'm not sure how easy that is right now.

Comment 17

[Daniel Holbert [:dholbert]]

(I filed bug 1498669 on the issue in the first part of Paul_B's video, BTW. Turns out that scenario was broken before + after this change, but with a different broken result before vs. after.)

Comment 18

[Victoria Wang [:victoria]]

Sounds like this bugzilla may contain an actual bug in addition to a discussion of what the behavior should be? Re: the latter:

For these combined item-containers, I suggested we should show the item accordion at top if navigating to it from a container accordion (or another item accordion). 

However, if the user selects an item-container in the markup view, I assume the user wants to primarily see that element as a container, so the container info should be at the top. This currently isn't the case in Nightly - I'm clicking on item-containers in the markup and seeing item accordions at the top. Is this something that is intended to be fixed?

(Also, not to overcomplicate things, but sounds like there could be a third case of doing everything in the markup and expecting results from actions in there. E.g. Selecting a flex container in markup, toggling on its 'flex' badge, then when clicking on its children, wanting to see item info first.)

Comment 19

[Victoria Wang [:victoria]]

To try to clarify my last comment a bit more:


1. When selecting an Item-Container via Markup view: I think we should change the current behavior to show the Container pane first. It's the most relevant to the page overlay, which seems like the most helpful initial thing to know about a flex-related layout. The higher level overview of the container pane also seems like a more easily understandable first impression for users who aren't necessarily aware of Flex features yet.

2. When navigating to an Item-Container from within the Layout panel: We'd show the Item pane first, as we do now.

(The third case mentioned above needs more testing/research - for now it seems safest to make the toggle behavior only about turning on the page overlay.)

Comment 20

[Gabriel Luong [:gl] (ΦωΦ)]

Temporally unassigning myself in case someone else has time to take on this bug before me.

Comment 21

[Patrick Brosset <:pbro>]

I have a patch that seems to address comment 19, but in some situations I'm having problems due to the tool refreshing many times in a row, making things a bit unpredictable. I think that perhaps bug 1507750 will help with this, so I'll wait for it to land first.

Comment 22

[Patrick Brosset <:pbro>]

Attachment: Bug 1496458 - Display the flex-container accordion first if a container+item is selected in markup-view; r?gl!

Flex elements can be both containers and items at the same time. When they get selected in the inspector
the sidebar shows both the container accordion and the item accordion.
This changeset makes sure the container accordion is displayed before the item accordion only when
the element is selected from the markup-view.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/00269c0edf56
Display the flex-container accordion first if a container+item is selected in markup-view; r=gl

Comment 24

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/00269c0edf56

Comment 25

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I reproduced this issue using 65.0a1(2018-10-04), on Windows 10 x64.
I can confirm this issue is fixed, I verified using Fx 65.0b11 on Windows 10 x64, macOS 10.13 and Ubuntu 16.04 LTS.
Cheers!