Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Revert theme and custom font family groups #63505

Open
richtabor opened this issue Jul 12, 2024 · 28 comments
Open

Proposal: Revert theme and custom font family groups #63505

richtabor opened this issue Jul 12, 2024 · 28 comments
Labels
[Feature] Font Library Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Regression Related to a regression in the latest release

Comments

@richtabor
Copy link
Member

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 #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 #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:

CleanShot.2024-07-12.at.16.50.30.mp4
@richtabor richtabor added [Type] Enhancement A suggestion for improvement. Needs Design Feedback Needs general design feedback. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Feature] Font Library labels Jul 12, 2024
@t-hamano
Copy link
Contributor

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
Copy link
Contributor

afercia commented Jul 15, 2024

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
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Contributor

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
Copy link
Contributor

afercia commented Jul 16, 2024

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Contributor

afercia commented Jul 18, 2024

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
Copy link
Contributor

Thanks for all your feedback.

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

  • Sidebar: Don't group fonts by source. Change the label to "Active fonts".
  • Font Library Modal: No changes
Sidebar Font Library Modal
image image

@richtabor
Copy link
Member Author

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.

@richtabor richtabor added this to Polish Jul 21, 2024
@t-hamano
Copy link
Contributor

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
Copy link
Contributor

afercia commented Jul 22, 2024

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
Copy link
Member Author

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
Copy link
Contributor

afercia commented Jul 23, 2024

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:

  • These are the active fonts.
  • Hey, there may be other inactive fonts already installed you can manage by clicking 'Manage fonts'.

@richtabor
Copy link
Member Author

richtabor commented Jul 27, 2024

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)?

@ndiego ndiego added the [Priority] High Used to indicate top priority items that need quick attention label Sep 17, 2024
@noisysocks
Copy link
Member

noisysocks commented Sep 17, 2024

@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
Copy link
Contributor

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
Copy link
Member

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

@noisysocks noisysocks removed the [Priority] High Used to indicate top priority items that need quick attention label Sep 18, 2024
@noisysocks noisysocks moved this from 📥 Todo to 🏈 Punted to 6.8 in WordPress 6.7 Editor Tasks Sep 18, 2024
@richtabor
Copy link
Member Author

richtabor commented Sep 23, 2024

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.

@richtabor richtabor moved this from 🏈 Punted to 6.8 to 📥 Todo in WordPress 6.7 Editor Tasks Sep 23, 2024
@richtabor richtabor added [Type] Regression Related to a regression in the latest release and removed [Type] Enhancement A suggestion for improvement. Needs Design Feedback Needs general design feedback. labels Sep 23, 2024
@richtabor richtabor moved this to Needs development in Polish Sep 23, 2024
@ndiego
Copy link
Member

ndiego commented Sep 23, 2024

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

@noisysocks noisysocks moved this from 📥 Todo to 🏗️ In Progress in WordPress 6.7 Editor Tasks Sep 24, 2024
@afercia
Copy link
Contributor

afercia commented Sep 24, 2024

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
Copy link
Member

ndiego commented Sep 24, 2024

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
Copy link
Contributor

afercia commented Sep 24, 2024

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:

  • Use icons + text whenever possible.
  • Use icon only exclusively when there's no space.

@afercia
Copy link
Contributor

afercia commented Sep 24, 2024

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:

  • These are the active fonts.
  • Hey, there may be other inactive fonts already installed you can manage by clicking 'Manage fonts'.

@afercia
Copy link
Contributor

afercia commented Oct 2, 2024

Related to #65590 (comment)

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:

  • Change the heading from 'Fonts' to 'Active fonts'.
  • Change the manage button text from 'Manage fonts' to 'Manage all fonts'.

@ndiego
Copy link
Member

ndiego commented Oct 9, 2024

Since #65590 have been merged, can this issue be removed from the 6.7 project board (or closed) @richtabor

@t-hamano t-hamano moved this from 🏗️ In Progress to 🏈 Punted to 6.8 in WordPress 6.7 Editor Tasks Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Font Library Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Regression Related to a regression in the latest release
Projects
Status: Needs development
Status: 🏈 Punted to 6.8
Development

No branches or pull requests

6 participants