|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
59 bytes, text/x-review-board-request
|Details | Review|
We recently set the min-width of flexible spacers to 0px which had the desired effect of making the toolbar shrink appropriately on narrow windows HOWEVER you can now get the spacers to shrink so small that they cannot be seen or used in Customize Mode. We need to change the min-width to 4px so that the spacers don't go away completely.
Whiteboard: [photon-structure] → [photon-structure] [triage]
Only in customize mode or also in "normal" mode?
On slack you also mentioned you want a max-width - what should that max-width be?
Priority: -- → P3
Whiteboard: [photon-structure] [triage] → [reserve-photon-structure]
Okay, we were able to test this out locally and resolved to set the min-width to 28px and the max-width to 112px. 28px is the size of an icon in the toolbar (icon + padding), and 112px comes from allowing for 4 icon slots (28x4). Note: when I set this locally I noticed a bug in Customize Mode that the URL bar changed widths and the flexible spacers appeared not to fill all the available space. I assume this is because I was hacking on values locally but just wanted to raise it as something to look for.
(In reply to Aaron Benson from comment #3) > Okay, we were able to test this out locally and resolved to set the > min-width to 28px That's only in customize mode, right?
(In reply to :Gijs from comment #1) > Only in customize mode or also in "normal" mode? As Florian implied, I'd still like to know the answer to this. If we're effectively going back to having a significant min-width outside of customize mode, this will re-introduce the issues in bug 1389505. I'm assuming we only want to do this inside customize mode, also given that the URL bar's *contents* are unimportant in customize mode (it's empty and disabled, after all), but that isn't the case outside customize mode.
This is for customize mode and 'live' mode. It still alleviates the problem we had before because the min-width is less -- 28px instead of 40px. 28px is arguably a better min-width because it's the width of just one toolbar item so it retains some dragspace in the toolbar. I realize it's not a complete fix for very narrow windows but there's more we can do in the URL bar itself to make the experience better (later on). Gijs, you're right that the URL bars contents aren't important, but I'd just point out that representing its size and position accurately is very important. We need to avoid a situation where the toolbar in customize mode looks different than 'live' mode because it works a little bit as a WYSIWYG editor.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Iteration: --- → 57.2 - Aug 29
Component: Menus → Toolbars and Customization
Summary: Flexible Spacers should have a min-width of 4px → Flexible Spacers should have a non-0 min-width
Comment on attachment 8898202 [details] Bug 1390327 - tweak min/max-width of the flexible spacers some more, https://reviewboard.mozilla.org/r/169570/#review174936 With enough twiddling I hope we will find the right balance here. rs=me
Attachment #8898202 - Flags: review?(jaws) → review+
Flags: qe-verify? → qe-verify+
QA Contact: gwimberly
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/71b57a23ee96 tweak min/max-width of the flexible spacers some more, r=jaws
Status: ASSIGNED → RESOLVED
Last Resolved: 6 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
(In reply to Aaron Benson from comment #3) > Okay, we were able to test this out locally and resolved to set the > min-width to 28px and the max-width to 112px. 28px is the size of an icon in > the toolbar (icon + padding), and 112px comes from allowing for 4 icon slots > (28x4). I believe that now leads to situations like bug 1391998, but I see from comment 5 and 6 that it was already understood that the approach in this bug can't work correctly by itself.
This made the spacers on our screenshots machines larger, which I presume was intended: https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=978f68c17245c37283a3635629efff66d2cdcba9&newProject=mozilla-central&newRev=8febdfacc7160c9ba90ba480e8bf5cd174e4fcab
I have reproduced the issue mentioned in comment 0 using an affected Firefox 57.0a1 build (BuildId:20170811100330). I have verified that the issue is not reproducible using Firefox 57.0b7 (Build Id:20171009192146) on Windows 10 64bit, macOS 10.11.6 and Ubuntu 16.04 64bit.
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified
You need to log in before you can comment on or make changes to this bug.