WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.55k stars 4.22k forks source link

Proposal: Revert theme and custom font family groups #63505

Open richtabor opened 4 months ago

richtabor commented 4 months ago

At first glance, grouping theme and custom fonts seems like a good idea (as implemented in this pr), but after exploring a bit further, I don't think it conceptually makes sense to do so.

Why?

Only the fonts that are active on my site render within the Typography panel, regardless of if they are fonts provided by the theme, or fonts that I added myself. The source of the font does not matter—only that it is active.

Adding this grouped distinction of theme and custom fonts, without a clear technical reason to have this distinction (other than colors have theme and custom groupings), does not support making WordPress more intuitive.

An argument could be made that colors should also loose this distinction, moving instead to the UX that fonts held prior to https://github.com/WordPress/gutenberg/pull/63211. What difference does it make to an end user if a color on a site is provided by the theme, augmented by the user, or a completely custom color?

I think that distinguishing theme vs. custom fonts makes it seem like I cannot disable theme fonts (like colors work) although it is clearly possible in the Font Library. When disabled, the font is no longer rendered in the "Theme" fonts group.

In an effort to push WordPress to be more intuitive, I propose that https://github.com/WordPress/gutenberg/pull/63211 is reverted back to a singular fonts group that renders all active fonts.

Visual

In the visual below, I can disable a font from the theme fonts grouping:

https://github.com/user-attachments/assets/24414088-0be3-41b1-8aef-39d2ecc05600

t-hamano commented 4 months ago

Thanks for the suggestion.

The reason for grouping fonts by source is:

  • To help users understand which source a font comes from without having to open the font library.
  • To improve consistency with other presets (Color, Shadow).

Personally, I prefer the current UI where they are grouped by source, just like the other presets (color, shadow, font size).

Any suggestions, @colorful-tones, @jasmussen, @afercia?

afercia commented 4 months ago

To me, what matters most is consistency. first Fonts are gouped by source in the modal dialog. As such, it's consistent, and expected, that they are grouped in the panel list as well. If the grouping doesn't make sense conceptually, then I would argue the grouping should be removed from any UI including the modal dialog. However, personally I do think the grouping is useful to visually scan the list of fonts and is consistent with other UIs.

jasmussen commented 4 months ago

I'd tend to agree with Rich: the delineation over source makes sense in the font library itself (the modal), but the inspector context is the context of the document, and should simply show all the fonts that are active.

richtabor commented 4 months ago

Fonts are gouped by source in the modal dialog. As such, it's consistent, and expected, that they are grouped in the panel list as well. If the grouping doesn't make sense conceptually, then I would argue the grouping should be removed from any UI including the modal dialog.

The difference is that you setting a font inactive removes it from what is effectively the active fonts list in the Typography panel. The modal shows you all fonts, installed, provided by the theme, or installable.

That nuance is key.

richtabor commented 4 months ago

Personally, I prefer the current UI where they are grouped by source, just like the other presets (color, shadow, font size).

There's a bit of nuance. For color, font sizes and shadows a user cannot remove the active shadows. If they could, I think we should remove the "theme" vs. "user" connotation there as well; making them simply what styles are active across the site.

t-hamano commented 4 months ago

For color, font sizes and shadows a user cannot remove the active shadows

For default and theme presets, this is true: for all colors, shadows, and font sizes, you can only change the values, you cannot delete them.

Similarly, the Font Library doesn't allow you to delete theme fonts, but instead lets you "deactivate" them. So in the sidebar, it will appear as if the font has been deleted, even if it hasn't. I think this difference is what makes UI decisions difficult.

If the grouping doesn't make sense conceptually, then I would argue the grouping should be removed from any UI including the modal dialog.

I think it's necessary to keep theme fonts and custom fonts separate in the modal dialog.

Otherwise, if users want to remove all custom fonts, they will have to click on every font one by one to see if it's a custom font, i.e. if the delete button exists.

For visibility, I will also send a ping to @matiasbenedetto

afercia commented 4 months ago

I'd tend to agree with Rich: the delineation over source makes sense in the font library itself (the modal), but the inspector context is the context of the document, and should simply show all the fonts that are active.

I'm not sure I agree. The editor is already complicated for users to understand. Consistency is key. As a user, I would not understand why fonts are grouped in the modal dialog and are not in the Inspector list.

The difference is that you setting a font inactive removes it from what is effectively the active fonts list in the Typography panel. The modal shows you all fonts, installed, provided by the theme, or installable.

Then, if that's the intent of the list in the inspector, I would argue the UI is not clear and should clearly communicate to users "hey, this is the list of currently activated fonts".

But, even clarifying that, visually the UI wouldn't be consistent with the modal dialog.

richtabor commented 4 months ago

I think it's necessary to keep theme fonts and custom fonts separate in the modal dialog.

Yes, I agree with this. The modal shows you all fonts, regardless of activation.

richtabor commented 4 months ago

But, even clarifying that, visually the UI wouldn't be consistent with the modal dialog.

They serve two different purposes with the same data.

One shows the fonts available on your site (I don't think "active" is a necessary distinction) while the other shows all fonts, whether installed by you, provided by the theme, or installable. They're not one and the same, so they should not be presented one and the same.

The separation of theme and custom fonts in the Typography panel introduces confusion, because we're presenting the two purposes into one.

afercia commented 4 months ago

One shows the fonts available on your site (I don't think "active" is a necessary distinction) while the other shows all fonts

I could agree to make these two UIs have a clear different purpose. But then, the UIs in the Typography panel should be labeled accordingly to inform users that's the list of available (or 'active') fonts. And that there may be more installed fonts in the Library that aren't active. The UI doesn't clarify this distinction. It should.

As a user, I don't understand what is the difference between this list:

Screenshot 2024-07-18 at 14 45 40

and this list:

Screenshot 2024-07-18 at 14 46 02

t-hamano commented 4 months ago

Thanks for all your feedback.

Given what we've discussed so far, what would be the ideal UI?

Sidebar Font Library Modal
image image
richtabor commented 4 months ago

Sidebar: Don't group fonts by source. Change the label to "Active fonts".

I'm confident visible fonts in this list imply that they are "active" font. The same as any other UI we have. Like colors for example, if it's listed, it's available.

t-hamano commented 4 months ago

Thanks for the feedback.

There are currently three ongoing PRs regarding the font library that would conflict with the code changes needed to resolve this issue, so I would like to address this issue once those three have been merged.

afercia commented 4 months ago

I'm confident visible fonts in this list imply that they are "active" font. The same as any other UI we have. Like colors for example, if it's listed, it's available.

I kindly disagree. To me, it's not clear what this list is. Also, for accessibility any section of settings should be identified by a meaningful heading. Please let's add a heading, thanks.

richtabor commented 4 months ago

No where else do we denote an "active" list of items, when there are no "inactive" items in a list. If you identify fonts in a list (just like when you identify colors), they are a list of available items.

And we don't use "active" in the interface copy anywhere. I don't want to complicate the UI further by adding arbitrary context.

Now, if we had a list of inactive fonts, then yes "Active fonts" is a meaningful heading. (we don't need a list of inactive fonts.)

afercia commented 4 months ago

I don't want to complicate the UI further by adding arbitrary context.

I guess this is the point where our views differ. Yes the editor UI is complicated. To me, users struggle with the UI because of the lack of context. Often, the UI doesn't explain what a certain item or object is. While for some users the visuals may be sufficient to explain an UI purpose, for many other users the lack of text or headings that identify the 'what' is a barrier.

by adding arbitrary context

In what way a heading that clearly explains what this UI is about would ever add 'arbitrary context'? Rather, it would add meaningful, specific context. I'm afraid this kind of design considerations keep into account only the visual aspect, which is not what I'd like to see in a project that aims to be accessible and usable for everyone. Information, content structure, semantics are way more important.

For what is worth, reverting this UI to exactly what it was before is a no from me. The current UI is unclear. As a user I don't understand what this list of fonts is about because there isn't any context that explains it.

As I said earlier, I'm not opposed to use this list to communicate the list of active fonts that can be used in the editor. But then the UI should clarify communicate:

richtabor commented 4 months ago

As a user I don't understand what this list of fonts is about because there isn't any context that explains it.

Do you feel the same way about color palettes, shadows, font sizes - any group of available presets to use throughout the site? That if they’re present, it maybe doesn’t mean they’re available (active)?

noisysocks commented 2 months ago

@t-hamano: Is this urgent for 6.7? It doesn't look like we'll make the RC cut off tomorrow but I can bless the issue if we think it's important.

t-hamano commented 2 months ago

I don't think this is an urgent issue for 6.7.

This issue is not a bug report, it's just for discussing whether the current UI is ideal and should be reverted, no consensus has been reached yet.

Personally, I prefer the current UI. I think one way to move this discussion forward would be to release the current UI as is into WP6.7 and get feedback from users.

noisysocks commented 2 months ago

Ok sounds good. If feedback is strong we can fix during beta period.

richtabor commented 2 months ago

Personally, I prefer the current UI. I think one way to move this discussion forward would be to release the current UI as is into WP6.7 and get feedback from users.

We should not change something with intent to change it back. I feel strongly this makes the experience more complicated than it's worth and that this should not be released to core.

I consider this a regression.

ndiego commented 2 months ago

I much prefer the current implementation in 6.6 to the one proposed in Gutenberg. It's also not clear in the Gutenberg implementation that those items listed are fonts.

6.6 Gutenberg
image image

Concerning the icon versus the button, I understand that icons reduce discoverability. However, the settings icon is consistent with other areas of the UI and, in my opinion, is similar to the three vertical dots in the options menus. Once you know what the icon is used for, the consistency makes the UI intuitive.

In fact, I would like to see something like the following for the situation where there are no fonts. This is consistent with the way you add filters to the Query block. Once a font is added, the + icon would switch to the settings icon.

Current Proposed
image image
afercia commented 2 months ago

In fact, I would like to see something like the following for the situation where there are no fonts. This is consistent with the way you add filters to the Query block. Once a font is added, the + icon would switch to the settings icon.

Icon-only buttons are a problem in the UI and do provide a reduced level of accessibility. They should be avoided when possible. The fact they're used broadly in the editor doesn't make them more accessible, In fact, that's a pattern that is consistently not accessible. I can't support a design where proliferation of unnecessary icon buttons introduces barriers for users. Wherever possible, the editor shoul duse buttons with visible text. Otherwise, the UI will actually exclude_ users instead of be based on an inclusive design. Thanks.

ndiego commented 2 months ago

Icon-only buttons are a problem in the UI and do provide a reduced level of accessibility. They should be avoided when possible. The fact they're used broadly in the editor doesn't make them more accessible, In fact, that's a pattern that is consistently not accessible.

I still think having a consistent UI is paramount here.

That said, we have an Editor preference that is intended to solve this, but it is not being applied everywhere it should. In the screenshot below, many of the icon buttons are replaced with text links, but none of those in the Settings Sidebar are. Perhaps we need a dedicated project to ensure all icon buttons in the sidebar have text alternatives when the "Show button text labels" setting is enabled 🤔

Preference Panel Editor
image image
afercia commented 2 months ago

I still think having a consistent UI is paramount

I'd totally agree. Unfortunately, the editor UI is largely inconsistent in many other places, including the placement of many icons that varies depending on the component and it's always unpredictable.

To accomodate for the 'Show button text label' preference, the design should be changed in the first place because there's not enough space to show the label text. I think I've pointed out in other issues that any new design should also take into account the 'Show button text label' preference and provide mockups for it. That should be an integral part of the design process and guidelines.

It is not happening though. There's no focus on the 'Show button text label' preference and design and development often miss to even test for it. Frankly, when switching to 'Show button text label' the UI appears in a very poor state.

That said, I do think that proliferation of icon-only buttons is not just an accessibility issue. It's also an usability issue and to me it's a wrong design. Icons only may be necessary in limited cases e.g. inside toolbars. But, filling the UI with icon only buttons isn't a pattern that helps all users. It may work for some users, it doesn't for others, regardless of their level of ability.

Icons do add value when accompanied with visible text. All the large web applicationsl, operating systems UI, mobile UI that focus on usability and accessibility do that:

afercia commented 2 months ago

Regarding the pending PR that reverts the UI change, I can only repeaet what I proposed earlier. I do recognize there is some value in using this list to communicate the list of active fonts

For what is worth, reverting this UI to exactly what it was before is a no from me. The current UI is unclear. As a user I don't understand what this list of fonts is about because there isn't any context that explains it.

As I said earlier, I'm not opposed to use this list to communicate the list of active fonts that can be used in the editor. But then the UI should clarify communicate:

afercia commented 1 month ago

Related to https://github.com/WordPress/gutenberg/pull/65590#issuecomment-2388544201

A quick improvement for 6.7 would be to better clarify that the list of fonts only lists the active ones and that there may be more available fonts but they are inactive.

I'd like to propose two string changes:

ndiego commented 1 month ago

Since https://github.com/WordPress/gutenberg/pull/65590 have been merged, can this issue be removed from the 6.7 project board (or closed) @richtabor