Closed Bug 1494949 Opened 6 years ago Closed 5 years ago

Flexbox-Toggle has off-by-one focus error

Categories

(DevTools :: Inspector, defect, P3)

63 Branch
defect

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)

Attached image flex toggle focus state
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 :)
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)
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)
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.
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)
See Also: → 1505704
Summary: Flexbox-Toggle has off-by-one focus error and misses a tooltip → Flexbox-Toggle has off-by-one focus error
Attached image ddg_toggle.gif
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)
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)
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.
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)
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)
> 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).
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: nobody → rcaliman
Status: NEW → ASSIGNED
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
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

Tried to land with unsupported CSS property. Fixed and queuing to land again.

Flags: needinfo?(rcaliman)
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
Whiteboard: [qa-66b-p2]
You need to log in before you can comment on or make changes to this bug.