1513922 - Use -moz-appearance: textfield for the Weight number input

Status: RESOLVED FIXED

(DevTools :: Inspector, enhancement, P3)

Description

[Florens Verschelde :fvsch]

Attachment: linux-screenshot.png

The Weight <input type="number"> looks rather ugly on Linux (see screenshot).

We should perhaps use `-moz-appearance: textfield` for this field, as we already do for Size and Line Height, to be consistent and more visually pleasing.

If it looks and works great on Windows and macOS, we can consider setting that for Linux only, but I would rather use the same style for all platforms.

Comment 1

[Razvan Caliman [:rcaliman]]

Attachment: Number input on Fonts Panel in Dark Theme on macOS.jpg

I'm in favor of hiding the increment/decrement arrows for those number inputs. The functionality is preserved when using up/down keys on the keyboard, similar to the inputs in the Rule view.

The number input is an ugly duckling on all platforms. A bit less on macOS by virtue of the Fonts panel being developed on that platform, but still not *really* integrated into the design. Some hacks were needed for the steppers to look better in the dark theme. They never vertically aligned properly either.

Those stepper buttons look bad on Windows and are downright inaccessible in High Contrast mode due to another Firefox bug. 
See: https://bugzilla.mozilla.org/show_bug.cgi?id=1496436

With Victoria and Martin's permissions, I'd be happy to see Florens' suggestion applied to all platforms.

Comment 2

[Martin Balfanz [:mbalfanz]]

Agreed.  The windows version doesn't look as bad, but is still unusable.  Changing -moz-appearance works for me.

Comment 4

[Victoria Wang [:victoria]]

Ah, that's too bad - I'm not sure many people are used to the increment/decrement keyboard behavior. From the Windows screenshots (non-high-contrast mode) I felt it's not so ugly that it distracts from the whole panel (like this linux example :)). 

I can see how this change would simplify things though, so I think it's fine to move forward on this.

Comment 5

[Martin Balfanz [:mbalfanz]]

Attachment: arrows-windows.png

For reference I added a Windows screenshot.

Comment 6

[Razvan Caliman [:rcaliman]]

Thank you both for confirmations!

This is an easy fix. I'll mark this as a good first bug for a newcomer to try and fix and set myself as a mentor.
Learn here how to get started with contributing to Firefox DevTools: 
https://docs.firefox-dev.tools/getting-started/
and
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch

The fix for this issue would have to be made in devtools/client/themes/fonts.css 
As Florens suggested, it's something along the lines of:
```
.font-value-input[type=number] {
  -moz-appearance: textfield;
}
```
This would target all number inputs in the Font Editor.

Also, please take care to remove the hacks made for positioning the number spin-box and adapting it for dark theme:
https://searchfox.org/mozilla-central/source/devtools/client/themes/fonts.css#268-276

Comment 7

[Victoria Wang [:victoria]]

Re: Windows screenshot, interestingly it looked less bad in the MDN screenshot which I assumed was Windows: https://mdn.mozillademos.org/files/16162/font-weight_cropped.png but 

I can see how here in Martin's screenshot, it does look basically unusable due to being so tiny/short. 

Sounds great to make this a good first bug!

Comment 8

[Sakshaat Choyikandi]

Hi Razvan, I am looking for a first bug to start with, would it be possible for me to contribute to this? Thanks!

Comment 9

[Garvit Khatri [:garvitdelhi]]

Attachment: Use -moz-appearance: textfield for the Weight number input

Comment 10

[Razvan Caliman [:rcaliman]]

@Garvit, usual etiquette on Bugzilla is that you first request to be assigned to a bug (or assign yourself if you can), then you work on it. This signals to others that may be interested in working on the same bug, as with Sakshaat above.

But since this among your first contributions and you already did the work to setup the dev environment (which is by no means trivial) and to fix the bug, I'll accept your patch on this bug. Thank you for your work on it!

@Sakshaat, I'm sorry for the situation on this bug. But if you're still interested in contributing to DevTools (we're very thankful for that), here's a list of other bugs marked as good-first-bug: https://mzl.la/2CNH1lB

Perhaps you can find another one there that interests you.

Even if bugs are assigned to someone, if you see inactivity for a long time since the last comment, you can ask to get the bug reassigned to you.

Comment 11

[Garvit Khatri [:garvitdelhi]]

@Razvan, I do realise it was indeed a mistake. I would be more aware in future before starting to work upon a bug.

@Sakshaat, I would like to apologize I didn't notice your comment and decided to work upon it after seeing it unassigned. I hope you will find a new bug to work upon soon.

Comment 12

[Noemi Erli[:noemi_erli]]

Unable to land this bug, the revision is blocked because it doesn't have the proper author information in Phabricator

Comment 13

[Garvit Khatri [:garvitdelhi]]

@Noemi what all information is needed/missing from Phabricator?

Comment 14

[Noemi Erli[:noemi_erli]]

I am only prompted with that message while trying to land from Phabricator, and there is a FAQ link about why this could be, maybe this can help you: https://wiki.mozilla.org/Phabricator/FAQ#Lando

Comment 15

[Razvan Caliman [:rcaliman]]

@Garvit, perhaps it is this issue from the FAQ linked above?

The revision was created via arcanist or moz-phab, but the error still appears.
This can happen if you have not set an author email in your .hgrc file (or git config).
Set your author email in your .hgrc to your username, Firstname Lastname <[e-mail]>.
Update your commit so it contains the new author data. Re-run arc diff so that the new commit+data is uploaded to Phabricator.

Comment 16

[Sakshaat Choyikandi]

@Garvit @Razvan no problem, I'll find another bug. Thanks.

Comment 17

[Garvit Khatri [:garvitdelhi]]

@Razvan Thanks I hope this would solve the problem. @Noemi I have updated and marking it again as checkin-needed. I hope it would work this time.

Comment 18

[Arthur Iakab [arthur_iakab]]

@Garvit Khatri tried to land the patch via Phabricator but it just hangs on the Preview Landing window and does not finalize the landing.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f42ed4eb7274
Use -moz-appearance: textfield for the Weight number input r=rcaliman

Comment 20

[Razvan Caliman [:rcaliman]]

I tried the Phabricator route and it seems to have worked.
This should land, unless there's a backout, which is not expected.

Comment 21

[Garvit Khatri [:garvitdelhi]]

@Razvan Thanks!

Comment 22

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/f42ed4eb7274