1491811 - Rules: replace custom checkbox with -moz-appearance:checkbox

Status: RESOLVED FIXED

(DevTools :: Inspector: Rules, enhancement, P2)

Description

[Florens Verschelde :fvsch]

Attachment: inspector-rules-checkbox.png

In a code review for bug 1490491, :pbro suggested removing the custom checkbox image used in Rules (for disabling declarations) and using the same design as in other tools (e.g. the Console or Network toolbars).

Trying it quickly on macOS, it seems to be working well. I’m not sure which size would be best:

- keeping the current 14x14 size, we get a 12x12 image
- with a 12x12 element, we get a 10x10 image

The smaller checkbox looks a bit better to me, but I’m not sure that the checkbox design for other OSes will work well at small sizes too.

Comment 1

[Florens Verschelde :fvsch]

If needed, we could use the standard-looking checkbox on some platforms, and fall back to the new SVG checkbox for others.

Victoria, do you have an opinion on this checkbox design, if we should try to use a more standard one and which size might work well?

Comment 2

[Tim Nguyen :ntim]

This might look out of place with the devtools dark theme, as it will be the only white element.

Comment 3

[Tim Nguyen :ntim]

(In reply to Tim Nguyen :ntim from comment #2)
> This might look out of place with the devtools dark theme, as it will be the
> only white element.

(I'm talking about -moz-appearance: checkbox, not using the SVG)

Comment 4

[Florens Verschelde :fvsch]

Tried `-moz-appearance: checkbox` quickly on Ubuntu, and unlike on macOS it doesn't pick up the `checked` attribute value, so the checkbox looks empty whatever its state. We probably need a real <input type=checkbox> for a good result.

> This might look out of place with the devtools dark theme, as it will be the
> only white element.

It does look a bit out of place, but we're already doing it for the Grid selector (Inspector > Layout), the class toggler (in Inspector > Rules), the Settings page, the Console and Network toolbars, etc.
So we may as well be consistent.

Apparently, macOS Mojave (10.14) dark mode comes with checkboxes and radio buttons which have a dark background. I wonder if there is something planned for that in Firefox.

Comment 5

[Tim Nguyen :ntim]

(In reply to Florens Verschelde from comment #4)
> Apparently, macOS Mojave (10.14) dark mode comes with checkboxes and radio
> buttons which have a dark background. I wonder if there is something planned
> for that in Firefox.

The macOS dark mode is not always in sync with the devtools dark mode.

Comment 6

[Florens Verschelde :fvsch]

I was just wondering if at some time Firefox would be able to paint native-looking checkboxes in their “dark” variant, and if we could use that.

Comment 7

[Victoria Wang [:victoria]]

I love the look of the 12px! Especially if we even make the click target a bit larger ;D. Not worried about doing native checkboxes in Ubuntu if it's a better experience.

Filed a dark mode empty checkbox issue for later: https://github.com/devtools-html/ux/issues/32

Comment 8

[Florens Verschelde :fvsch]

Related:

- The new checkbox image from bug 1490491 landed
- In bug 1190113 we added a left border to the clickable area, so that clicks on the left side of the checkbox activates it

If we end up using a native checkbox, and perhaps make it small (e.g. 12px), we should consider having a larger element around it (to the left, but also a few pixels to the top/bottom and right) that catches clicks.

Comment 9

[Florens Verschelde :fvsch]

We can use real <input type="checkbox"> elements, keep the native style if it looks alright, and give it a custom style with -moz-appearance: none if it doesn't.

Comment 10

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Screenshot 2019-10-15 at 16.40.41.png

I decided to go with the 12px version because Victoria likes it.

@victoria does this work for you?

Comment 11

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b785d8a36ebffed28b4244fa66c9927a1350750

Comment 12

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Bug 1491811 - Rules: replace custom checkbox with -moz-appearance:checkbox r?fvsch!

Comment 13

[Victoria Wang [:victoria]]

Nice, thanks for working on this Mike! Couple tweaks:

• Needs more spacing between the checkbox and the text (match fvsch's mockup above)
• Labels should be 1px lower in relation to the checkbox

Comment 14

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Screenshot 2019-10-16 at 11.43.54.png

@victoria I have overlayed this new version in photoshop. The checkbox and text positions match perfectly.

The line height in the mockup is much smaller in our tools but I decided to ignore that as I think our line heights are good.

Is this okay?

Comment 15

[Florens Verschelde :fvsch]

Attachment: firefox-checkbox-size-gnome-fedora.png

On Linux with Gnome, at least with Fedora's default theme (might be different on Ubuntu, for instance), it looks like native checkboxes can be scaled up but now down.

Their minimum size seems to be 16px.
Our line height in Rules is not always fixed, but it's often 15px or 16px.

Comment 16

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a0514690751e7eee9b495f217159222352ebd06a

Comment 17

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

It may just be that you need to go down to 8px for the checkbox to resize.

Either way we just need to look at it and see if we need to tweak anything.

The try push should [hopefully] give us a screenshot on each of our target OSs.

Comment 18

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Ubuntu.png

Ubuntu

Comment 19

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: windows.png

Comment 20

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: OSX.png

Comment 21

[Florens Verschelde :fvsch]

The line-height looks tall on Ubuntu. I'm not sure what the DPR was, but with a few guesses and calculations it kinda looks like 17px or maybe 18px? I wonder if that might come from the default margins for input[type="checkbox"] (if they're not reset here and we're not using absolute positioning).

The macOS checkboxes look smaller overall. It looks like they render with 1px of blank space around the graphic, so a width: 12px; height: 12px; checkbox is 10x10 visually. We can fix it using different styles (10px for everyone, 12px for macOS), or keep it like that if we don't mind the difference.

Comment 22

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

(In reply to Florens Verschelde :fvsch from comment #21)

The line-height looks tall on Ubuntu. I'm not sure what the DPR was, but with a few guesses and calculations it kinda looks like 17px or maybe 18px? I wonder if that might come from the default margins for input[type="checkbox"] (if they're not reset here and we're not using absolute positioning).

I can try resetting the top and bottom margins etc.

The macOS checkboxes look smaller overall. It looks like they render with 1px of blank space around the graphic, so a width: 12px; height: 12px; checkbox is 10x10 visually. We can fix it using different styles (10px for everyone, 12px for macOS), or keep it like that if we don't mind the difference.

Victoria likes that size and it is her call. I have to admit that I think they match the default text size perfectly on OSX.

Comment 23

[Victoria Wang [:victoria]]

Attachment: image.png

I might be confused about something, but looks like the line height is taller in all the screenshots? (Like 4px taller than the current style?) It would be helpful to see a bigger screenshot that shows several rules.

Victoria likes that size and it is her call.

Thanks, but to clarify, I do often defer this kind of thing to Florens because they are really good at the details 🙂

Comment 24

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=16640caf11d25668ed8fcc95f5c65d0e9634cb73

Comment 25

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: OSX.png

Comment 26

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Linux.png

Comment 27

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Win.png

Comment 28

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Linux.png

Comment 29

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Win.png

Comment 30

[Florens Verschelde :fvsch]

To sum up discussions on Slack:

• not 100% sure a HTML checkbox will render well at small sizes on Windows and Linux (especially Linux)
• we want to try to roll out the native checkboxes on macOS first, and see if we can enable Windows and Linux later after more testing

Comment 31

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

@fvsch So now we have the resized native checkboxes on OSX and the SVG checkboxes on Windows and Linux... I am happy with this, especially as we have improved accessibility at the same time.

I hope you are happy to review this... otherwise please ping me and let me know.

Comment 32

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a8c318edef5078169c2fad21bafd3864af7c03c0

Comment 33

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

fvsch wants me to try this again but using -moz-appearance:none on the checkboxes to allow resizing outside of OSX.

Lets give it a try.

Comment 34

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e67fd094d0a0a02d2025d3f541ca6f7b038f690a

Comment 35

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

We are now using -moz-appearance:none to apply the SVG checkboxes under Windows and Linux and using the native checkboxes on OSX.

Comment 36

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=db2635698922edadf5737afe59eca631ef1ce613

Comment 37

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ad2fdebc212c7a12ca86f95577b565a3573d6fe

Comment 38

[Florens Verschelde :fvsch]

Super happy to see this shaping up. Re. questions you had about -moz-appearance: none making the checkbox invisible: this is probably because of the deleted line in devtools/client/jar.mn: if we don't restore it, Firefox doesn't find the checkbox image.

Comment 39

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c682b1c0b8ac3a242779a594641076f9a9225253

Comment 40

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ee9569a447c199e17c906d74dd32b2fab8d2e0b5

Comment 41

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=da8fb49bec062fb4746fe1c6868894478d3510c9

Comment 42

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e69e9a6aa261
Rules: replace custom checkbox with -moz-appearance:checkbox r=fvsch

Comment 43

[Andreea Pavel [:apavel]]

Backed out for failing dt at browser_inspector_textbox-menu.js and browser_rules_edit-selector-commit.js

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=272992243&resultStatus=testfailed%2Cbusted%2Cexception&revision=e69e9a6aa2610da603e511b1e6075926aeb46168

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=272992243&repo=autoland&lineNumber=13337
and https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=272992292&repo=autoland&lineNumber=6689

Backout: https://hg.mozilla.org/integration/autoland/rev/cf0b83b2948807e14afa9bbae298800b814c1792

Comment 44

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e9782fcde44dfa824f047ab35a32e12eea5c4264

Comment 45

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5b0caada1999
Rules: replace custom checkbox with -moz-appearance:checkbox r=fvsch

Comment 46

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/5b0caada1999

Comment 47

[Julian Descottes [:jdescottes]]

There seems to be a small performance regression on some inspector tests, and it might be linked to this bug:

• alert https://treeherder.mozilla.org/perf.html#/alerts?id=23608
• pushlog https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=519b6a47aa5c129c6566b5dcd954f0d75d4a091b&tochange=2521937495c4611096a75ac684f77e1dea610ca7

regression: 2-4 % on select/deselector and improvement on expandall manychildren.

The regression is small and there is also an improvement. Not necessarily worth a follow up, but maybe you have an idea about what might have caused this and want to look into it.

Comment 48

[Julian Descottes [:jdescottes]]

(see comment above)

Comment 49

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

We also observed this.

On OSX we are now using native checkboxes but there is less of a regression there.

In the CSS we use e.g. :root[platform="linux"] to identify the different OSes. We need to do this because Linux & Windows don't reliably resize checkboxes.

I guess :root[platform="mac"] etc. are slow selectors so I suppose that this is what is causing the regression.