Closed Bug 1586566 Opened 4 months ago Closed 4 months ago

After replacement of textbox with html:input, the spinbuttons on number boxes look bad on Windows

Categories

(Thunderbird :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 71.0

People

(Reporter: jorgk-bmo, Assigned: Paenglab)

Details

Attachments

(7 files, 5 obsolete files)

Attached image number-box.png

See attached screenshot taken from the image properties. But also looking bad in account settings, for example server settings for ports or anywhere else.

Basically the top button it missing a top border, or the whole widget doesn't fit into the box any more.

Flags: needinfo?(richard.marti)
Flags: needinfo?(khushil324)

Jörg for Windows review and Alessandro for Linux and Mac review.

Alessandro, I moved the spinbuttons on Linux a bit to the right with a negative margin. What do you think about it?

Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Flags: needinfo?(richard.marti)
Attachment #9099273 - Flags: review?(jorgk)
Attachment #9099273 - Flags: review?(alessandro)
Comment on attachment 9099273 [details] [diff] [review]
1586566-input-number-tweaks.patch

Review of attachment 9099273 [details] [diff] [review]:
-----------------------------------------------------------------

Overall I think this is the right approach, but I'm seeing some visual inconsistencies.
As you can see from the screenshot above, the number fields in the server settings area have the spinners a bit too attached to the end, so I'm not sure about the negative margin.

In another tab tho, that CSS doesn't seem to apply, even if the file is loaded and other styles apply properly.
Attachment #9099273 - Flags: review?(alessandro) → feedback+

This is why I specially noted the margin change. Linux with its many different themes isn't so easy in such areas.

And yes, not all AM pages had the input-fields.css imported when they did the html:input change. Fixed.

Attachment #9099273 - Attachment is obsolete: true
Attachment #9099273 - Flags: review?(jorgk)
Attachment #9099346 - Flags: review?(jorgk)
Attachment #9099346 - Flags: review?(alessandro)
Attached image numberbox-Linux.png

This is how it looks on my Linux without the smaller margin on the right.

Flags: needinfo?(khushil324)
Comment on attachment 9099346 [details] [diff] [review]
1586566-input-number-tweaks.patch

Review of attachment 9099346 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me on Linux and macos.
I understand Richard's point, and totally agree that styling for Linux is complicated since every DE can apply its own style.
In general, I think it's better to avoid negative values to perfectly align elements if it's not consistent across distros.
Attachment #9099346 - Flags: review?(alessandro) → review+
Attached image number box.png

I guess my first complaint is that the top button doesn't have a top edge. It doesn't have on in TB 68, but it does have one in TB 60.

The set of two buttons should sit pretty "snug" on the right side of the input box. There should be zero or one pixel to at the top, right and button to the input box border.

With the patch, the buttons have moved so far down that their bottom edge is actually outside the input field, that's even uglier than before when only the space around it was badly distributed (and the top border missing).

I looked again: On TB 68 in the account settings you have 2px at the top, right and left. There are no spin buttons in the image properties.

On trunk you now have 2px on the top, 2 px to the right, and 1px at the bottom. And in the image properties, the buttons overlap the field border on the top, there are 2px gap on the right, and it undershoots the border by one 1px at the bottom, as I said in my previous comment.

The issue with the too small boxes in the image properties dialog was that bug 1580512 didn't apply in this dialog because messenger.css isn't loaded there. Moved the rule from messenger.css to input-fields.css.

No change on Linux and Mac files.

Attachment #9099346 - Attachment is obsolete: true
Attachment #9099346 - Flags: review?(jorgk)
Attachment #9099465 - Flags: review?(jorgk)

Oops, I should have applied the patch :-(

It still differ because the ones that converted the boxes didn't always add the input-inline class. Only on Windows is a difference visible.

Comment on attachment 9099465 [details] [diff] [review]
1586566-input-number-tweaks.patch

Review of attachment 9099465 [details] [diff] [review]:
-----------------------------------------------------------------

Better, but still not pixel-perfect.

::: mail/themes/windows/mail/input-fields.css
@@ +5,5 @@
>  @import url("chrome://messenger/skin/shared/input-fields.css");
>  @namespace html url("http://www.w3.org/1999/xhtml");
>  
> +html|input.input-inline {
> +  padding: 2px 2px 1px;

This should either be 1 1 1, or just 0. I think 0 is best.

For the picture dimensions, it then sits exactly in the number box. The same for the port (143) in the IMAP server settings. I have no idea why the input box for the "check for messages every XX min" box is taller, so we get 2px top/right/bottom for the spinbuttons, the same for the SMTP port.

Can those number boxes all be made the same height?

I checked on TB 68. There image properties don't have spinbuttons, but all the number boxes in the AM have 2px all around.

So if you want to match TB 68, you need to make it padding: 2px 2px 2px;

This is my proposal. padding 0, if you omit it, you get padding 1.

Also fixed the inline port which is the only inline port in the system:
https://searchfox.org/comm-central/search?q=port.*inline&case=false&regexp=true&path=.xul

and one of the very few inline numbers:
https://searchfox.org/comm-central/search?q=number.*inline&case=false&regexp=true&path=.xul

So we should fix that inconsistency.

Attachment #9099491 - Flags: feedback?(richard.marti)

Didn't refresh :-(

Attachment #9099491 - Attachment is obsolete: true
Attachment #9099491 - Flags: feedback?(richard.marti)
Attachment #9099497 - Flags: feedback?(richard.marti)

I added the input-inline class to all AM panes. and used padding: 0;.

Attachment #9099465 - Attachment is obsolete: true
Attachment #9099497 - Attachment is obsolete: true
Attachment #9099465 - Flags: review?(jorgk)
Attachment #9099497 - Flags: feedback?(richard.marti)
Attachment #9099501 - Flags: review?(jorgk)
Comment on attachment 9099501 [details] [diff] [review]
1586566-input-number-tweaks.patch

Yes, that certainly looks better than my suggestion. Landing now.

Can we still add a top border to the upper spinbutton. Or is that a fixed SVG from M-C?
Attachment #9099501 - Flags: review?(jorgk) → review+

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/e4dff5cebc88
Tweaks for the input type="number" to make the spinbuttons align better. r=aleca,jorgk DONTBUILD

Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 71.0
Attached image spinbutton.png

(In reply to Jorg K (GMT+2) from comment #20)

Can we still add a top border to the upper spinbutton. Or is that a fixed
SVG from M-C?

The button has a -moz-appearance: spinner-upbutton;. Where is a light border that has the same colour as the button body. When I try to set border-top-color: black; I get the result you see on the top of the image. :-(

There is a light border that has the same colour as the button body.

Right, and there is no point having a border which is has same colour as the area it's trying to delimit.

This needs to be a toolkit bug to fix it but it's very unlikely as FX doesn't, or maybe very very seldom, use this buttons. We could try to override them, but in a new bug and Windows only.

You need to log in before you can comment on or make changes to this bug.