Flexible Spacers should have a non-0 min-width

VERIFIED FIXED in Firefox 57

Status

()

Firefox
Toolbars and Customization
P1
normal
VERIFIED FIXED
6 months ago
4 months ago

People

(Reporter: Aaron Benson, Assigned: Gijs)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 verified)

Details

(Whiteboard: [reserve-photon-structure])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

6 months ago
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.

Updated

6 months ago
Whiteboard: [photon-structure] → [photon-structure] [triage]
(Assignee)

Comment 1

6 months ago
Only in customize mode or also in "normal" mode?
Flags: needinfo?(abenson)
(Assignee)

Comment 2

6 months ago
On slack you also mentioned you want a max-width - what should that max-width be?
(Assignee)

Updated

6 months ago
Blocks: 1389505

Updated

6 months ago
Flags: qe-verify?
Priority: -- → P3
Whiteboard: [photon-structure] [triage] → [reserve-photon-structure]
(Reporter)

Comment 3

6 months ago
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.
Flags: needinfo?(abenson)
(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?
(Assignee)

Comment 5

6 months ago
(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.
Flags: needinfo?(abenson)
(Reporter)

Comment 6

6 months ago
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.
Flags: needinfo?(abenson)
(Assignee)

Updated

6 months ago
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
(Assignee)

Updated

6 months ago
Iteration: --- → 57.2 - Aug 29
Component: Menus → Toolbars and Customization
Comment hidden (mozreview-request)

Updated

6 months ago
Priority: P3 → P1
(Assignee)

Updated

6 months ago
Summary: Flexible Spacers should have a min-width of 4px → Flexible Spacers should have a non-0 min-width

Comment 8

6 months ago
mozreview-review
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+
(Assignee)

Updated

6 months ago
Blocks: 1387512
Flags: qe-verify? → qe-verify+
QA Contact: gwimberly

Comment 9

6 months ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/71b57a23ee96
tweak min/max-width of the flexible spacers some more, r=jaws

Comment 10

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/71b57a23ee96
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.
(Assignee)

Updated

6 months ago
Depends on: 1392631
(Assignee)

Updated

5 months ago
Duplicate of this bug: 1389796
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
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.