Closed Bug 1640417 Opened 4 years ago Closed 4 years ago

Improve visual styling of font dropdown in reader mode

Categories

(Toolkit :: Reader Mode, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla78
Tracking Status
firefox-esr68 --- unaffected
firefox76 --- unaffected
firefox77 --- unaffected
firefox78 --- fixed

People

(Reporter: soeren.hentzschel, Assigned: Gijs)

References

(Regression)

Details

(Keywords: regression)

Attachments

(4 files)

Attached image screenshot

While I like the new reader mode design the font settings of new reader mode design looks totally broken.

  • The buttons have the small "circle" control of radio input elements. This looks totally out of place and causes that the user interface feels broken. The old reader mode design used a visually much better way to indicate the active state. This circle should not be visible at all.
  • While all other elements are horizontally centered the "circle" is left-aligned. This does not fit at all other parts of the design.
  • The labels of the color option sticks to the top left of each button and are not, as expected, in the middle of these buttons.
  • Some labels are to the right of the "circle", but not all. It's probably caused by the fact that other languages than English have larger words and the width is fixed. See the color options in the attached screenshot for a German build of Firefox.
  • The grey elements with the current values (font size for example) looks unbalanced because of the big width and the small height.
  • The text in the grey elements with the current values is not correctly aligned (sticks to the top of the grey element, not in the middle).

The following were already problems with the old design but if you're already on it…

  • All buttons has a hover but not the color options.
  • The arrow of the panel has a border that does not fit the border of the panel itself.

(In reply to Sören Hentzschel from comment #0)

While I like the new reader mode design the font settings of new reader mode design looks totally broken.

This is not a helpful or objective way to give feedback, so I'm rephrasing the bug summary. "totally broken" is what you say when things fail to work at all or you get big black blobs on screen because of graphics issues or because we forgot to include some images or something. This is not "totally broken".

  • The buttons have the small "circle" control of radio input elements. This looks totally out of place and causes that the user interface feels broken. The old reader mode design used a visually much better way to indicate the active state. This circle should not be visible at all.

Well, they are radio buttons. You can only select one of the 2/3 at the time. In any case, it was a deliberate design feature, not a mistake or an accident. They are a different control type to the other elements in the dialog, so having some visual indication of this is helpful.

  • While all other elements are horizontally centered the "circle" is left-aligned. This does not fit at all other parts of the design.
  • The labels of the color option sticks to the top left of each button and are not, as expected, in the middle of these buttons.

Center-aligning radio buttons including variable-length text labels always looks messy (because the items have different widths, so they get have different horizontal starting points when centered), so this doesn't seem fixable as long as we keep the radio buttons.

  • Some labels are to the right of the "circle", but not all. It's probably caused by the fact that other languages than English have larger words and the width is fixed. See the color options in the attached screenshot for a German build of Firefox.

This is a bug, but yes, it's only like this in locales with longer labels.

  • The grey elements with the current values (font size for example) looks unbalanced because of the big width and the small height.
  • The text in the grey elements with the current values is not correctly aligned (sticks to the top of the grey element, not in the middle).

These seem like comparatively very minor points, and the first of them is really just a matter of opinion...

The following were already problems with the old design but if you're already on it…

  • All buttons has a hover but not the color options.

As you say, this was already the case.

  • The arrow of the panel has a border that does not fit the border of the panel itself.

The arrow is sort of a funny one; It's got the right (non-opaque) colour but the stroke gets applied on top of a transparent background, meaning that it blends with the panel shadow (making it appear darker) in a way that the rest of the panel's border doesn't (that gets applied against the white panel background - and the panel background was slightly darker before, making it perfect white was a last-minute adjustment...), which is why it looks off in your screenshot. I don't really know how best to fix it, but I also think this is barely noticeable if you're not looking for it...

Abe, what do you want to do about the radio buttons and their labels (esp. when they don't fit), the current value items, and hover styling for the color options, if anything?

Flags: needinfo?(awallin)
Summary: font settings of new reader mode design looks broken → Improve visual styling of font dropdown in reader mode

(In reply to :Gijs (he/him) from comment #1)

(In reply to Sören Hentzschel from comment #0)

While I like the new reader mode design the font settings of new reader mode design looks totally broken.

This is not a helpful or objective way to give feedback, so I'm rephrasing the bug summary. "totally broken" is what you say when things fail to work at all or you get big black blobs on screen because of graphics issues or because we forgot to include some images or something. This is not "totally broken".

I am okay with the change of the bug title but I don't think that my comment was not helpful or unobjective feedback at all because I didn't said "the design looks broken" and was done. I took the time to explain why it feels broken to me. I apologize if you took it that way, but that was not my intention at all. Yes, the feature is not "totally broken" but this was not what I said. The design of this feature feels "totally broken" to me because it looks as the style or parts of the style of this panel are missing. This is my impression and what i tried to explain.

Well, they are radio buttons. You can only select one of the 2/3 at the time. In any case, it was a deliberate design feature, not a mistake or an accident. They are a different control type to the other elements in the dialog, so having some visual indication of this is helpful.

I know that they are radio buttons. But this is a implemenation detail and doesn't mean that it has to look like unstyled radio buttons. As I already said: There was also a visual indication in the old design. And before you say that a line is not a good visual indiciator: The devtools use exactly the same for indicating the selected tool and subpanel (the only difference is the position: top vs bottom). Maybe we should even go a step further: use the same position and color for the line as in the devtools tabs to have a consistent design.

Center-aligning radio buttons including variable-length text labels always looks messy (because the items have different widths, so they get have different horizontal starting points when centered), so this doesn't seem fixable as long as we keep the radio buttons.

Well, then we have another reason not to use visible radio buttons. It just looks like a bug if everything is centered except one thing.

These seem like comparatively very minor points, and the first of them is really just a matter of opinion...

Sure, these are minor points and almost everything is a matter of a opinion, not only these points. But I didn't want only mention the "big issues" but everything that can be improved in my opinion. It's still up to you and your colleagues what to change and what not.

But the first of these two items is not only a matter of my opinion it's even different than in the design specification (https://mozilla.invisionapp.com/share/87N9YQYCTJZ#/screens/315073983). The aspect ratio is totally different in reality compared to the mockup.

As you say, this was already the case.

Sure. But is there a reason not to have a hover effect? It's not different than the serif vs sans-serif option and that option has a hover effect. If we already change everything should existing issues not be fixed? Especially if no big change of code is needed. ;-)

I don't really know how best to fix it, but I also think this is barely noticeable if you're not looking for it...

Well, as a website developer with an eye for details I don't have to look for it, things like that actually jump out at me. But at least it's less noticeable as in the old design, so it's an improvement… :D

I agree that the radio buttons look weird. I think you can dispense with those, center the text and continue to use the blue stroke around the border to show the active state.

The fonts also look undersized to me on Linux, so I would increase the size of those.

Set release status flags based on info from the regressing bug 1550836

There are some improvements we could make here. @Gijs can we do the following?

  • Because different languages can cause an overrun of the text in the boxes with radio buttons let's remove those and center the text. We should increase the width of the border to 2px to make the selected state more obvious.

  • The circle with values between controls should have a smaller width. Reduce to 36px

  • Vertically align text in the circles

See attached mockup.

Flags: needinfo?(awallin)
Attached image Type control menu.png
Attached image hover.png

For hover state let's try a 2px border that sits a couple pixels below the box. For consistency this can be applied to both Sans-serif/Serif and Light/Dark/Sepia. See

Making this a P3 for now - though if Gijs takes this on, we should make it a P1.

Severity: -- → S4
Priority: -- → P3

Hey Gijs, do you have time on your plate for Abraham's suggestions in comment 5, comment 6 and comment 7?

Flags: needinfo?(gijskruitbosch+bugs)

Yeah, it's on my radar.

Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Flags: needinfo?(gijskruitbosch+bugs)
Priority: P3 → P1

This fixes the following issues:

  • hover/active colours now look better on dark theme (match main toolbar lwtheme styles)
  • radio-type items now have custom styling
  • radio-type items no longer use buttons, only input[type=radio] with a subsequent label
  • hover/active/selection styling for the radio items is improved
  • cleans up unused CSS variables
  • styling of the 'current value' boxes
  • removes display:none hr elements
  • uses classes for each 'row' container to simplify the CSS
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/9b06c0ec158c improve reader mode font dropdown styling, r=jaws
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
QA Whiteboard: [qa-78b-p2]
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: