As seen in the attached new mockup, there are a number of small changes we'd like to make the flexbox inspector so it's easier to use: - Add a "Flex Items" header before the list of items - Highlight the corresponding item in the page when hovering over an item in the list (I think this is actually bug 1495775) - Add a "Overlay" label next to the highlighter toggle so people understand what it is for - Remove the Flexbox container properties section at the bottom, and replace them with little badges next to the flex container Rep for: direction, wrap and alignment. Although I would note that we have a big project coming up for alignment later, so that last badge doesn't necessarily have to land.

Thanks! I had trouble loading Invision yesterday but here's the link in case you want to see it in context with the other mockups. https://mozilla.invisionapp.com/share/MZNA228P5WB#/screens/319386528

gl: In case it helps here's the inspect link https://mozilla.invisionapp.com/d/#/console/14988591/319386528/inspect

2 more things that aren't currently in line with the latest mockups: - hovering over flex items in the list of items should highlight the whole row, not just the element Rep - larger/bolder numbers for flex items

(In reply to Patrick Brosset <:pbro> from comment #3) > - hovering over flex items in the list of items should highlight the whole > row, not just the element Rep I should also note that clicking anywhere on the row should open the flex item sizing panel, not just when you click on the Rep.

A couple polish items from https://discourse.mozilla.org/t/flexbox-inspector-input-wanted/30611/18? - would be great if the width of the sibling dropdown could expand larger to fit larger element names, e.g. large as 90% of the space - but still centered with a left-aligned arrows icon :D - Non overlapping labels but I'm sure you're already aware of this :)

One more thing I wanted to mention, based on me navigating around between items and containers: When viewing a flex item accordion, if we won't be able to overlay the sizing diagram in-page like this: https://mozilla.invisionapp.com/share/MZNA228P5WB#/screens/311896910 I feel we should display a basic highlight over the item in-page like this: https://mozilla.invisionapp.com/share/MZNA228P5WB#/screens/319386528 Currently it seems easy to lose the context of where the item is in the page when drilling down into an item.

Also, a polish item for the accordion headers that start with "Flex item of..." - truncate long headers so they only take up one line. (new.reddit.com has a selector named header._2GyPfdsi-MbQFyHRECo9GO.cx1ohrUAq6ARaXTX2u8YN .aswboo-0.isJDXQ.s1czsxkv-0.bYsyvY)

We decided to hold off on adding the "Overlay" text until Victoria determines if this is useful or if we can keep it the same from her UX research. that is being conducted this week.

Comment on attachment 9023082 [details] [diff] [review] Part 2: Add badges to show the flex-direction and flex-wrap properties of the flex container. [1.0] Review of attachment 9023082 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/client/inspector/markup/views/element-editor.js @@ +286,5 @@ > }, > > _createEventBadge: function() { > this._eventBadge = this.doc.createElement("div"); > + this._eventBadge.className = "markup-badge inspector-badge"; These 2 class names feel a bit redundant now. Could we live with just 1 class? inspector-badge? (and therefore rename all the --markup-badge variables to --inspector-badge)

Comment on attachment 9023082 [details] [diff] [review] Part 2: Add badges to show the flex-direction and flex-wrap properties of the flex container. [1.0] Review of attachment 9023082 [details] [diff] [review]: ----------------------------------------------------------------- One thing I forgot to note about this patch too is: I'd like to see at least one test that checks the existence of these badges in the DOM. And perhaps another one that checks their correctness (something that iterates over a few simple test cases with/without wrap and with different directions and checks the values displayed in the badges)

Comment on attachment 9023083 [details] [diff] [review] Part 3: Implement the new styles for the flex item list. Review of attachment 9023083 [details] [diff] [review]: ----------------------------------------------------------------- Code changes look good. And this is a good improvement. There are still a few minor things to polish up before we can land this: - The Flex Items label is not vertically aligned with Flex container Rep above it (and its badges). It's a few pixels to the left. - The item's numbers appear bigger and bolder on Victoria's mockup - The right arrowhead > to the right of each item is too close to the edge of the panel. In the mockups, there's a bit more spacing here.

Comment on attachment 9024914 [details] [diff] [review] Part 2: Add badges to show the flex-direction and flex-wrap properties of the flex container. [2.0] Review of attachment 9024914 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, this looks good to me. I only have 2 remaining questions/comments and I'd like a chance to re-review once answered/addressed: - would it make sense to create a new badge.css file for an even smaller import size? - see my comment below, it seems to me like we can clean some variables even more. ::: devtools/client/themes/markup.css @@ +6,5 @@ > --markup-badge-border-color: #CACAD1; > --markup-badge-color: var(--grey-60); > --markup-badge-hover-background-color: #DFDFE8; > --markup-badge-interactive-background-color: var(--grey-20); > --markup-badge-interactive-color: var(--grey-90); Why did you keep these variables here. Since we're moving the badge styling to inspector.css, it seems like these variables should be deleted too. Also, I can see that you've kept on using these specific markup-badge variables in this file, for the markup-expand-badge elements. But I don't understand why we can't use the inspector-badge variable there too now.

Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/06b4d9761d46 Part 1: Remove the FlexboxContainerProperties component. r=pbro https://hg.mozilla.org/integration/mozilla-inbound/rev/ad62c3439651 Part 2: Add badges to show the flex-direction and flex-wrap properties of the flex container. r=pbro https://hg.mozilla.org/integration/mozilla-inbound/rev/e7f8e9c7301b Part 3: Implement the new styles for the flex item list. r=pbro

Backed out 3 changesets (Bug 1498115) for failures in browser_flexbox_container_properties.js Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=e7f8e9c7301bc4cd4088bcf244ca55b877fbb034&selectedJob=211993244 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=211993244&repo=mozilla-inbound&lineNumber=4722 Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/4228a1200fdc057b021a1bd6a2fcd654c37b5c4d

Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/df843a2c7c75 Part 1: Remove the FlexboxContainerProperties component. r=pbro https://hg.mozilla.org/integration/mozilla-inbound/rev/bbcd0df86411 Part 2: Add badges to show the flex-direction and flex-wrap properties of the flex container. r=pbro https://hg.mozilla.org/integration/mozilla-inbound/rev/0051c8d339a9 Part 3: Implement the new styles for the flex item list. r=pbro

https://hg.mozilla.org/mozilla-central/rev/df843a2c7c75 https://hg.mozilla.org/mozilla-central/rev/bbcd0df86411 https://hg.mozilla.org/mozilla-central/rev/0051c8d339a9