Closed Bug 1578097 Opened 1 year ago Closed 1 year ago

The devtools.layout.flexbox.opened preference does not match the Layout UI

Categories

(DevTools :: Inspector: Layout, defect, P2)

defect

Tracking

(firefox71 fixed)

RESOLVED FIXED
Firefox 71
Tracking Status
firefox71 --- fixed

People

(Reporter: fvsch, Assigned: fvsch)

Details

Attachments

(1 file)

We save the "Flexbox" accordion's opened/closed state as a single preference (devtools.layout.flexbox.opened), but we have 3 different Flexbox-related accordions:

  • "Flexbox" (show a message: "Select a flex container or item to continue")
  • "Flex Container": when selected node is a flex container
  • "Flex Item of …": when selected node is a flex item

Currently, when we create the Layout tab, all these accordion items are rendered opened or closed by reading the value of devtools.layout.flexbox.opened. When the user opens or closes one of those accordion items, we also toggle the value of the devtools.layout.flexbox.opened preference.

Now that we fixed bug 1576604, this system works well when only showing one of the accordion items at a time, but when showing two (for nodes which are flex containers + flex items) the mismatch between the UI (two accordion items) and the preference (a single flag) can cause issues.

Scenario where the current implementation works

STR:

  1. In about:config, set devtools.layout.flexbox.opened to the default value (true).
  2. Go to https://codepen.io/fvsch/pen/yLBMEwe
  3. Right-click a pink circle and select "Inspect Element"
  4. In the Layout tab, close the "Flex Item of …" tab
  5. Close the toolbox (F12)
  6. Right-click a pink circle and select "Inspect Element"

Expected and actual result:

  • The "Flex Item of …" accordion item is closed

Open question:

  • If we add a step #7 which is to use the Markup view to select the parent element (blue square), what would users expect then? Should both "Flex Container" and "Flex Item of …" be closed? Or should "Flex Item of …" be closed, but "Flex Container" remains opened?

Scenario where it breaks

STR:

  1. In about:config, set devtools.layout.flexbox.opened to the default value (true).
  2. Go to https://codepen.io/fvsch/pen/yLBMEwe
  3. Right-click a blue square and select "Inspect Element"
  4. In the Layout tab, close the "Flex Container" tab
  5. Close the toolbox (F12)
  6. Right-click a blue square and select "Inspect Element"

Expected result:

  • The "Flex Container" item is closed
  • The "Flex Item of …" item is open

Actual result:

  • The "Flex Container" item is closed
  • The "Flex Item of …" item is closed

Solutions

To me this is an information architecture issue.
I can picture two different solutions:

  1. Put all the Flexbox-related content in a single accordion item ("Flexbox").
  2. Treat each Flexbox-related item as its own entity, with an associated preference:
    • devtools.layout.flexbox.opened
    • devtools.layout.flex-container.opened
    • devtools.layout.flex-item.opened
      (Or maybe use devtools.layout.flexbox.opened for both the empty message and the Flex Container section?)

In the short term, #2 looks more practical. Reorganizing the Flexbox content to fit in a single accordion item with sub-sections would require some UX work that's maybe best spent on exploring the Unified Layout Panel?

Hi Tim, following your bug report in bug 1546079 I fixed an accordion state issue in bug 1576604 (which can be tested in Nightly), and this bug would also fix some smaller issues with persisting the state of the Flexbox accordion items.

Do you think those combined fixes will fix the issues you had (in bug 1546079), and work out well for your use case?

Flags: needinfo?(ntim.bugs)

Martin, this is the Flexbox accordion issue I was talking about on Slack.

I made a patch (D44331) which implements the "use one preference per flexbox accordion item" approach. The prefs look like this in browser/app/profile/firefox.js:

// Whether or not the flexbox panel is opened in the layout view
pref("devtools.layout.flexbox.opened", true);
// Whether or not the flexbox container panel is opened in the layout view
pref("devtools.layout.flex-container.opened", true);
// Whether or not the flexbox item panel is opened in the layout view
pref("devtools.layout.flex-item.opened", true);

If having 3 prefs is overkill, we could perhaps consider that "Flexbox" (empty with text message) and "Flex Container" are the same accordion item and share the existing devtools.layout.flexbox.opened pref, and only create a new devtools.layout.flex-item.opened pref.

Test Plan

I tested my patch with this use case in mind:

  1. I don't want to see the "Flexbox" section because it's empty;
  2. I don't want to see the "Flex Container" section because I never use it and it pushes other content (e.g. "Box Model") after the fold;
  3. I do want to see the "Flex Item" section, which I find more useful.

After loading https://codepen.io/fvsch/pen/yLBMEwe, I closed the first two sections and kept "Flex Item" open. Then closed the toolbox, reopened Inspector, and focused different types of nodes. For all nodes, "Flexbox" and "Flex Container" remained closed, and "Flex Item" was opened.

Flags: needinfo?(mbalfanz)

(In reply to Florens Verschelde :fvsch from comment #1)

Hi Tim, following your bug report in bug 1546079 I fixed an accordion state issue in bug 1576604 (which can be tested in Nightly), and this bug would also fix some smaller issues with persisting the state of the Flexbox accordion items.

Do you think those combined fixes will fix the issues you had (in bug 1546079), and work out well for your use case?

As long as the collapsing state of each section is persisted (which seems to be what both bugs do), it should work for me.

Flags: needinfo?(ntim.bugs)

I agree with :ntim. I don't think 3 prefs is overkill, and I actually see benefits in only showing one of the flex panels. So, the change looks good to me

Flags: needinfo?(mbalfanz)
Assignee: nobody → florens
Status: NEW → ASSIGNED
Has Regression Range: --- → irrelevant
Has STR: --- → yes
OS: Unspecified → All
Priority: -- → P2
Hardware: Unspecified → All
Pushed by florens@fvsch.com:
https://hg.mozilla.org/integration/autoland/rev/4fc01a7061b6
Add prefs to persist the open states of flexbox accordion items separately; r=gl
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71
You need to log in before you can comment on or make changes to this bug.