Character counter CSS needs tweaks on Windows

RESOLVED FIXED in 1.6

Status

defect
--
minor
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: clokep, Assigned: clokep, Mentored)

Tracking

Details

(Whiteboard: [1.6-blocking][good first bug])

Attachments

(5 attachments, 2 obsolete attachments)

(Assignee)

Description

5 years ago
Posted image char-count-win.png
It looks like there's a gap between the counter and the edges of the textbox. The error border (the red border) also has a gap between it and the textbox edges on the left and right.
(Assignee)

Comment 1

5 years ago

Updated

5 years ago
Whiteboard: [1.6-blocking]

Comment 2

5 years ago
Don't forget to check the popup bar that appears when replying to a tweet as well. Thanks!
(Assignee)

Comment 3

5 years ago
I'm going to take this to not forget about it. But someone can feel free to steal this if they'd like.
Assignee: nobody → clokep
Mentor: clokep
Severity: normal → minor
Status: NEW → ASSIGNED
Whiteboard: [1.6-blocking] → [1.6-blocking][good first bug]
(Assignee)

Comment 4

5 years ago
Posted patch Fix v1 (obsolete) — Splinter Review
Uploading images in a second.
Attachment #8527176 - Flags: review?(aleth)
(Assignee)

Comment 5

5 years ago
(Assignee)

Comment 6

5 years ago
Posted image Char counter error with patch (obsolete) —

Comment 7

5 years ago
Comment on attachment 8527176 [details] [diff] [review]
Fix v1

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

Does aero need any specific/separate changes?

::: im/themes/conversation.css
@@ +340,1 @@
>    -moz-appearance: none;

I think we now have moz-appearance: none on all OS, so it can be moved to the earlier .conv-textbox rule for all OS in this file, simplifying the ifdef.

@@ -427,5 @@
>  }
>  
>  .splitter-bottom {
>    background: #ddd; /* match URL popup bottom edge from tabbrowser-*.css */
> -%ifdef XP_MACOSX

Doesn't removing this make an unintended change for Linux?
Attachment #8527176 - Flags: review?(aleth) → review-
(Assignee)

Comment 8

5 years ago
Posted patch Fix v2Splinter Review
Attachment #8527176 - Attachment is obsolete: true
Attachment #8527184 - Flags: review?(aleth)
(Assignee)

Comment 9

5 years ago
Attachment #8527178 - Attachment is obsolete: true

Comment 10

5 years ago
Comment on attachment 8527184 [details] [diff] [review]
Fix v2

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

Looks fine, thanks!

Looking at the screenshot, I suspect the splitter-bottom was higher on Windows to match the width of the splitter between the nicklist and the conversation? If you prefer to keep the splitter-bottom thicker, you could change its height only when the reply-to bar is shown (or simply not worry about that edge case). Go with what you think looks best for Windows.
Attachment #8527184 - Flags: review?(aleth) → review+
(Assignee)

Comment 11

5 years ago
Landed it with the 1 px border. We need to tweak the Windows UI more, but I think this is an improvement.

https://hg.mozilla.org/comm-central/rev/18ae00dd170f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.6
You need to log in before you can comment on or make changes to this bug.