Closed Bug 1338106 Opened 8 years ago Closed 8 years ago

Improve tab display when toolbox width is too small.

Categories

(DevTools :: Framework, enhancement, P3)

enhancement

Tracking

(firefox55 fixed)

RESOLVED FIXED
Firefox 55
Tracking Status
firefox55 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

Details

Attachments

(4 files)

For a toolbox width inferior to 800/900 pixels, the toolbox tabs start becoming hard to distinguish from one another. I feel like the icons are a nice addition when we can have the text displayed, but if only the icon is visible it becomes a bit to know which tool is which. These icons are very FF devtools specific and I think you need a lot of hours spent on the tool before you become familiar with them. Even I, who spends a lot of time with the tools, keep mixing the style editor icon and the debugger icon. I think we should value text more than icons when we don't have enough room for both. I can see two paths to improve this: 1. increase the minimum width for the tabs. This will make tab overflow more frequent, but we should soon have a button to access all tabs (via Bug 1335608) 2. remove text-overflow: ellipsis and hide the icons based on the toolbox viewport width. I attached a screenshot comparing the current state with a simple implementation of solution 2. cc-ing pbro and Honza as we discussed this on IRC earlier today.
I started Bug 1253880 awhile back to refresh the devtools-tab to look like Helen's design. I personally found that if we removed the icons and flex the tab sizes, we can actually squeeze alot more tabs together, but ultimately, we should move towards Helen's design that would have the dropdown ">>" to select different tabs.
(In reply to Julian Descottes [:jdescottes] from comment #0) > Created attachment 8835452 [details] > toolbox-toolbar-no-icons-comparison.png > > For a toolbox width inferior to 800/900 pixels, the toolbox tabs start > becoming hard to distinguish from one another. > > I feel like the icons are a nice addition when we can have the text > displayed, but if only the icon is visible it becomes a bit to know which > tool is which. These icons are very FF devtools specific and I think you > need a lot of hours spent on the tool before you become familiar with them. > Even I, who spends a lot of time with the tools, keep mixing the style > editor icon and the debugger icon. Precisely, I am experiencing the same. Icons tend to be cryptic in this case. > I think we should value text more than icons when we don't have enough room > for both. Yes > I can see two paths to improve this: > 1. increase the minimum width for the tabs. This will make tab overflow more > frequent, but we should soon have a button to access all tabs (via Bug > 1335608) > 2. remove text-overflow: ellipsis and hide the icons based on the toolbox > viewport width. Agree with both. The minimum width should be set so, the labels are fully visible. If there still isn't enough space some of the tabs can be hidden (starting from the right/last) and 'all tools' menu should appear at the end. We could yet experiment with left-right tab padding and see if we can save a few more pixels. I was yet suggesting scrolling (on IRC, I implemented that in Firebug) but, I don't think it's necessary at this point. Let's see how it works with the all-tools menu first. Thanks for working on this, it'll be great UI/UX improvement! Honza > I attached a screenshot comparing the current state with a simple > implementation of solution 2. > > cc-ing pbro and Honza as we discussed this on IRC earlier today.
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Comment on attachment 8844682 [details] Bug 1338106 - hide toolbox toolbar icons for narrow viewport; Switching to f? for now. This patch is an attempt at making the toolbox toolbar a bit more compact. I tried to avoid using media queries for now, I kind of like the fact that the UI smoothly transitions to remove the icons when you resize the devtools. Let me know what you think.
Attachment #8844682 - Flags: review?(ntim.bugs) → feedback?(ntim.bugs)
Comment on attachment 8844682 [details] Bug 1338106 - hide toolbox toolbar icons for narrow viewport; https://reviewboard.mozilla.org/r/118004/#review119834 ::: devtools/client/themes/toolbox.css:170 (Diff revision 1) > .devtools-tab > img { > + box-sizing: border-box; > border: none; > margin: 0; > - margin-inline-start: 10px; > + margin-inline-end: 5px; > + padding-inline-start: 5px; This padding seems unnecessary (it looks fine when removed) ::: devtools/client/themes/toolbox.css:173 (Diff revision 1) > margin: 0; > - margin-inline-start: 10px; > + margin-inline-end: 5px; > + padding-inline-start: 5px; > opacity: 0.8; > max-height: 16px; > - width: 16px; /* Prevents collapse during theme switching */ > + width: 26px; /* Prevents collapse during theme switching */ Can you remove this line ? The comment doesn't seem to apply anymore (I think it's leftover from the XUL tabs), and 26px seems arbitrary to me.
Attachment #8844682 - Flags: feedback?(ntim.bugs)
Thanks for the feedback. On IRC you mentioned a few things: - shrinking the icons looks buggy - would be nice to fade the text when the tab overflows - we could use smaller command buttons (24px instead of 28px) I switched to a media query to hide the icons now. In a second changeset I tried to emulate the text-overflow: fade; property which is not available yet. I find the implementation to be a bit complex (having to resort to both :before and :after pseudo to get a decent outcome when the tab background color is not fully opaque...). But on the other end it looks nicer than simply clipping the text. And we can always remove the code when text-overflow: fade; is ready.
(In reply to Julian Descottes [:jdescottes] from comment #8) > > I find the implementation to be a bit complex (having to resort to both > :before and :after pseudo to get a decent outcome when the tab background > color is not fully opaque...). But on the other end it looks nicer than > simply clipping the text. And we can always remove the code when > text-overflow: fade; is ready. Meh, my bad. I actually didn't think about mask-image. After checking the implementation used for the browser tabs, it looks like that's what is being used here. I just updated my implementation to use it, should be much more maintainable.
(for the record this will have to be rebased and slightly modified once I can get Bug 1345323 on central)
Comment on attachment 8844839 [details] Bug 1338106 - reduce min width of devtools toolbar command buttons to 24px; https://reviewboard.mozilla.org/r/118172/#review120156 It'd be nice to have the options tab match as well.
Attachment #8844839 - Flags: review?(ntim.bugs) → review+
Comment on attachment 8844682 [details] Bug 1338106 - hide toolbox toolbar icons for narrow viewport; https://reviewboard.mozilla.org/r/118004/#review120614 ::: devtools/client/themes/toolbox.css:113 (Diff revision 5) > + visibility: hidden; > + margin-inline-start: 5px; > + margin-inline-end: 0; > + width: 0; Doesn't display: none work instead of all this ?
Attachment #8844682 - Flags: review?(ntim.bugs)
Comment on attachment 8844682 [details] Bug 1338106 - hide toolbox toolbar icons for narrow viewport; https://reviewboard.mozilla.org/r/118004/#review120624
Attachment #8844682 - Flags: review+
Comment on attachment 8844833 [details] Bug 1338106 - Add mask-image to fade out text of devtools-tabs; https://reviewboard.mozilla.org/r/118166/#review120622 ::: devtools/client/themes/firebug-theme.css:100 (Diff revision 5) > > .theme-firebug .toolbox-tabs { > margin-bottom: -1px; > } > > -.theme-firebug .devtools-tab span { > +.theme-firebug .devtools-tab-label { Can you add a comment explaining why this is needed ? Something like: /* Set the end padding on the label to make sure the label gets faded out properly */ Or something better :) ::: devtools/client/themes/toolbox.css:113 (Diff revision 5) > } > > +.devtools-tab-label { > + mask-image: linear-gradient(to left, transparent 0, black 6px); > + min-width: 1px; > + padding-inline-end: 10px; Same here. ::: devtools/client/themes/toolbox.css:123 (Diff revision 5) > +} > + > /* Hide tab icons when the viewport width is limited */ > @media (max-width: 700px) { > + .devtools-tab-label { > + padding-inline-end: 5px; And here.
Attachment #8844833 - Flags: review?(ntim.bugs) → review+
Comment on attachment 8844839 [details] Bug 1338106 - reduce min width of devtools toolbar command buttons to 24px; https://reviewboard.mozilla.org/r/118172/#review120628 ::: devtools/client/themes/toolbox.css:321 (Diff revision 4) > padding: 0 !important; > margin: 0 !important; > border: none !important; > border-radius: 0 !important; > position: relative; > - min-width: 28px; > + min-width: 24px; Marking an issue here: Can you fix the toolbox options button to be the same width as the toolbox buttons ?
Comment on attachment 8844839 [details] Bug 1338106 - reduce min width of devtools toolbar command buttons to 24px; https://reviewboard.mozilla.org/r/118172/#review120628 Thanks for the review, all your comments should be addressed in the last version! > Marking an issue here: Can you fix the toolbox options button to be the same width as the toolbox buttons ? Sure, fixed to 24px as well now.
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b1eeaade0307 hide toolbox toolbar icons for narrow viewport;r=ntim https://hg.mozilla.org/integration/autoland/rev/1396c6c9a248 reduce min width of devtools toolbar command buttons to 24px;r=ntim https://hg.mozilla.org/integration/autoland/rev/18a136d5ca3f Add mask-image to fade out text of devtools-tabs;r=ntim
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: