Closed
Bug 1494949
Opened 6 years ago
Closed 6 years ago
Flexbox-Toggle has off-by-one focus error
Categories
(DevTools :: Inspector, defect, P3)
Tracking
(firefox66 fixed)
RESOLVED
FIXED
Firefox 66
Tracking | Status | |
---|---|---|
firefox66 | --- | fixed |
People
(Reporter: nachtigall, Assigned: rcaliman)
References
(Blocks 1 open bug)
Details
(Whiteboard: [qa-66b-p2])
Attachments
(6 files)
The on/off toggle when focussed looks strange. The blue background color is thicker at bottom than at top, looks weird. Focus does not only happen, when TABbing to the toggle but also after clicking on it with the mouse.
I guess the top and bottom border should be same size. Now the top is 1px less than bottom.
see screenshot
Also, I had no idea what this toggle does. I guess you should add a `title` attribute so that a tooltip is shown. (Same for the clickable `flex` bubble in the inspector).
> Also, I had no idea what this toggle does.
fwiw, I know now: It adds some "helper lines" around the element in the browser. Great feature, but looks like it needs some final polishing :)
Comment 2•6 years ago
|
||
Thanks for filing Jens. Yes it does need more polish, and we're actively working on this. We're hoping to ship this with either 64 or 65 once the quality is where we want it.
Thanks for the feedback so far.
On macOS with a retina display, I don't see this problem. Do you mind specifying which OS and which monitor type you are using? That would be helpful.
Flags: needinfo?(nachtigall)
Priority: -- → P3
> On macOS with a retina display, I don't see this problem. Do you mind specifying which OS and which monitor type you are using?
I see this off-by-one px on both Windows 10 (plain old 24″ Samsung monitor) and Ubuntu 16.04 (even older, 22″ Asus monitor).
From my perspective, best way of getting this fixed, would be, if, well, you, could provide me new fancy retina monitors (4K sufficient), but I admit that this might not scale to your whole user base ;)
Flags: needinfo?(nachtigall)
Assignee | ||
Comment 4•6 years ago
|
||
The toggle focus state is set with CSS box-shadow's spread-radius value:
https://searchfox.org/mozilla-central/source/devtools/client/themes/common.css#829
I guess we can try to increase that to alleviate any artifacts, but I don't have a non-HiDPI monitor to check the results.
@florens, do you have a recommendation?
Flags: needinfo?(florens)
Comment 5•6 years ago
|
||
I am indeed seeing a blurry shadow on a @1x display.
The toggle itself renders with the correct dimensions (32x16 pixels, no fractional values involved), and theoretically the shadow should draw a perfect 1px "border" around the element, but it doesn't work.
I'm getting imperfect results with a simple div too, e.g.:
div {
width: 10px;
height: 10px;
box-shadow: 0 0 0 1px black;
}
I expect this code to give me a perfect #000 border, but I'm getting things like a dark gray on the top or bottom part of the shadow.
This might be specced in some way, or a consequence of the rendering pipeline (this is with WebRender off, by the way, and I'm also worried that WebRender might make things slightly worse).
My advice would be to use a border instead. The downside of using a border is that it will be painted on top of the background, so we need to account for that in the design.
Comment 6•6 years ago
|
||
I did a test using `border: solid 1px transparent` by default, and changing the border-color on :focus.
(By the way, do we want to use :focus, or :-moz-focusring to avoid showing that outline on mouse clicks?)
If we use a 12px "thumb", 2px padding, and 1px of border, it looks quite nice, arguably even better than currently. And that border is crisp.
Flags: needinfo?(florens)
Summary: Flexbox-Toggle has off-by-one focus error and misses a tooltip → Flexbox-Toggle has off-by-one focus error
I split the "title attribute aka tooltip missing part" into another bug 1505704 (maybe tag it as "easy" or "first good bug").
As for this bug here I am not sure whom to NI here...
Florens has given some good advise on how to proceed. Not sure who's the product owner but maybe someone from the team could reply to this.
Secondly, fwiw and reference I think the DDG homepage https://duckduckgo.com/?q=firefox&t=h_&ia=web has a much nicer toogle because:
1. Shows ✓
2. Grays out when inactive
See attached GIF.
I have problems seeing the state (on or off) of these sliding toggles and the Firefox Devtools is no different here. Since https://design.firefox.com/ makes no assumptions here, maybe ask them or go ahead (I've seen that the "Content blocking" toggle from the Hamburger menu has also gone in Firefox Nightly now)
Flags: needinfo?(rcaliman)
Comment 8•6 years ago
|
||
Two design questions:
1. Is it okay if we ever-so-slightly tweak the toggle's design to add a 1px border than can be used for the focus style? That means that visually there would be a 3px padding between the background and the thumb (vs 2px currently).
2. Jens suggests adding a "check mark" graphic element to the active state, like DuckDuckGo does (screenshot: https://screenshots.firefox.com/sKtxtuGRrkI4Albb/duckduckgo.com), so that the active state is not shown only through color and position. Do you think that is worth exploring?
Flags: needinfo?(victoria)
Comment 9•6 years ago
|
||
I believe we also wanted to make the whole toggle smaller and more align with what we had in the Firefox hamburger menu seen in Firefox release only now. The easiest way to do that would probably to examine the styles used in the hamburger menu.
Assignee | ||
Comment 10•6 years ago
|
||
Thank you for the advice, Florens!
I see no problem with replacing the box shadow with a 1px border that gets colorized on focus. I have a small patch ready for that, but I'll wait for Victoria's reply to see if the toggle needs additional adjusting.
Flags: needinfo?(rcaliman)
Comment 11•6 years ago
|
||
Sorry for the delay in responding here!
1. Does this mean that the switch background would look 1px larger on all sides? I think this might look too large - I'd prefer a solution where the focus ring appears as 1px around the current shape of the switch. Would be happy to look at what you're working on however!
2. Yes, this seems like a really good idea. While I try to match Firefox, this seems like a good place for us do better, and I will try to convince Firefox UX to follow suit :D. We could start with this icon for the check https://design.firefox.com/icons/viewer/#check
Flags: needinfo?(victoria)
Comment 12•6 years ago
|
||
> I'd prefer a solution where the focus ring appears as 1px around the current shape of the switch
The issue is that technical solutions for focus rings outside the switch don't work well:
- CSS outlines are always rectangular
- And the box-shadow we used is buggy (maybe that's a known graphics bug)
Borders are more reliable, but they appear "inside" the element (on top of the background-color).
Comment 13•6 years ago
|
||
Ah I see! I might be confused - could we just use an inset border within the current switch shape, rather than enlarging the switch? I suppose you want to preserve extra px of the background color but if I'm understanding correctly I don't think it would look bad to lose that. (I'm assuming we'll use some kind of matching lighter blue border color that is visible in the on-switch state)
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → rcaliman
Status: NEW → ASSIGNED
Assignee | ||
Comment 14•6 years ago
|
||
Updated•6 years ago
|
Blocks: dt-flexbox
Comment 15•6 years ago
|
||
Pushed by rcaliman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6a2a0437e8e9
Replace toggle box shadow with border to highlight focused state; r=fvsch
Comment 16•6 years ago
|
||
Backed out changeset 6a2a0437e8e9 (Bug 1494949) for failures in browser_parsable_css.js
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=221699229&repo=autoland&lineNumber=1427
Backout: https://hg.mozilla.org/integration/autoland/rev/0651e1f9c1ed13d0fd94d665d6fa5f086db87c0a
Flags: needinfo?(rcaliman)
Comment 17•6 years ago
|
||
Pushed by rcaliman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4a87188c418f
Replace toggle box shadow with border to highlight focused state; r=fvsch
Assignee | ||
Comment 18•6 years ago
|
||
Tried to land with unsupported CSS property. Fixed and queuing to land again.
Flags: needinfo?(rcaliman)
Comment 19•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox66:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
Updated•6 years ago
|
Whiteboard: [qa-66b-p2]
You need to log in
before you can comment on or make changes to this bug.
Description
•