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

Unable to reset only font size from Typography block settings #38219

Open
Clorith opened this issue Jan 25, 2022 · 17 comments
Open

Unable to reset only font size from Typography block settings #38219

Clorith opened this issue Jan 25, 2022 · 17 comments
Labels
[Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Type] Bug An existing feature does not function as intended

Comments

@Clorith
Copy link
Member

Clorith commented Jan 25, 2022

Description

When choosing a font size in the new Typography selector, there is no way to return to the default value, without resetting all typography settings.

For comparison, the letter case setting lets you return to the default value by tapping the selected value once more to un-select it.

Initially reported at https://wordpress.org/support/topic/wordpress-5-9-beta-2-cant-reset-font-size/

Step-by-step reproduction instructions

  1. Open a post
  2. Select a paragraph block
  3. In the block settings, choose a font size in the selector
    4.There is now no way to un-select

Screenshots, screen recording, code snippet

No response

Environment info

  • WordPress 5.9-RC4

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@annezazu annezazu added [Type] Enhancement A suggestion for improvement. [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Type] Bug An existing feature does not function as intended and removed [Type] Enhancement A suggestion for improvement. labels Jan 26, 2022
@jasmussen
Copy link
Contributor

Is this flow working for you?
reset font

There seems to be a bug with the panel, though, as the "plus" icon should be an ellipsis when there's a property there already, but I could be missing something. CC: @ramonjd @aaronrobertshaw

@aaronrobertshaw
Copy link
Contributor

Thanks for the ping @jasmussen and for submitting the issue @Clorith 👍

There seems to be a bug with the panel, though, as the "plus" icon should be an ellipsis when there's a property there already, but I could be missing something. CC: @ramonjd @aaronrobertshaw

I think there might be some confusion over when the panel's icon should be a plus or a vertical ellipsis.

The icon was changed to include the plus icon in #34107 with the aim to encourage users to explore that menu for additional options when all optional controls are hidden.

This code change illustrates that intent.

The primary use case for this was when the panel's initial state would be empty due to all controls being optional. In the case of the typography panel however the font-size picker is a default control and is shown all the time. As such it doesn't currently impact which icon (plus or vertical ellipsis) is shown.

I'm not sure what the best course of action is here. I can appreciate the desire to encourage exploration of other options within the panel's menu, particularly when they are not displayed by default. In addition to this, neither the plus or the vertical ellipsis icons indicate that the control can be reset via the menu.

This dual-purpose nature of the menu in the ToolsPanel design has been an ongoing point of discussion (example 1, example 2, among others).

Is this flow working for you?

The ability to reset only the font-size works for me. I believe the issue here is the assumed knowledge of the panel's design as mentioned above.

My suggestion would be to close this issue and open one specific to improving the UX of the ToolsPanel.

@Clorith
Copy link
Member Author

Clorith commented Jan 26, 2022

That's not a very discoverable route if I have to hit the plus (to me indicating "add a new field"), then hitting the minus next to the attribute i want to clear.

For comparison, if you add the Letter Case attribute, you can click the same value twice to un-select it, which is a much more discoverable, and even an established pattern, in selection-scenarios.

I'm still thinking this is its own bug, and not part of an overarching ToolsPanel UX overhaul, as the behavior differs between the elements with the same kind of control.

@aaronrobertshaw
Copy link
Contributor

That's not a very discoverable route if I have to hit the plus (to me indicating "add a new field"), then hitting the minus next to the attribute i want to clear.

I agree. The discoverability of this could be improved and the use of the plus icon refined. Perhaps it would be better for it to only be used in place of the vertical ellipsis when the panel would be completely empty. cc @ramonjd for thoughts

My understanding of why the reset functionality was moved to the items within the panel's menu, is that as we got more and more controls available in the sidebar it became overly cluttered with reset buttons. For a control that could be toggled off without the need for a separate reset button, the menu item should be just an extra method of resetting the control's value.

For comparison, if you add the Letter Case attribute, you can click the same value twice to un-select it, which is a much more discoverable, and even an established pattern, in selection-scenarios.

The difference between the underlying components for the font-size picker and letter case controls do highlight the problem you are describing.

The font-size picker is more than just the toggle group, which is essentially a radio button group under the hood hence not being able to deselect. It also needs to handle possibly complex CSS values, long lists of preset font-size options, or custom values. This means a font-size selection might have been made via the custom value field or select control (depending on font-size presets) and couldn't be toggled off by deselecting something back in the toggle group view (if available).

In contrast, the letter-case control is simple and its possible values are pretty static allowing for the ability to toggle a selection off.

I believe @ntsekouras would know more about the reasoning behind the current iteration of the font-size picker as well as all the tricky edge cases it must withstand.

Perhaps we could rename this issue and update its description to better reflect the problem being discussed?

As it stands currently, we can reset only the font size from the typography block settings.

@ntsekouras
Copy link
Contributor

I agree with all of you that the discoverability/handling of reset could be improved. In general we should aim for consistency and as far as I know the plan is to also change the other control like letter case to use the same internal component (ToggleGroupControl) with font size picker.

The font-size picker is more than just the toggle group, which is essentially a radio button group under the hood hence not being able to deselect.

I don't believe there would be any problems regarding the font size control's values, but this comes down to the ToggleGroupControl that's being used. I'm not aware of any accessibility issues that may exist with having a radio group 'deselect' as an option or some other implication but I'd like the thoughts of @ciampo or @mirka about this.

@aaronrobertshaw
Copy link
Contributor

I know the plan is to also change the other control like letter case to use the same internal component (ToggleGroupControl) with font size picker.

I was aware of this being raised previously and at the time being dismissed (at least temporarily) because of issues that radio button groups introduce. It would be good if the ToggleGroupControl could allow the toggle off behaviour out of the box and therefore be used more widely in these controls as proposed. Even if that means conditionally not using the RadioGroup component internally.

I don't believe there would be any problems regarding the font size control's values, but this comes down to the ToggleGroupControl that's being used.

My comments regarding the values and presets were meant more to indicate that the font-size picker is more than just a toggle group control and so depending on how the picker is currently being displayed you might not have the toggle group to even toggle off as a means of resetting.

I'm not aware of any accessibility issues that may exist with having a radio group 'deselect' as an option or some other implication

I was of the impression that once a radio group selection had been made, normal user interaction couldn't toggle off any selection. Also, our RadioGroup component's readme states that it should have a default option already selected which we can't do without adding extra options to the control and also gaining access to merged theme.json and global styles within the block editor.

I'm likely missing something that others working in this area have already considered so happy to defer.

@ciampo
Copy link
Contributor

ciampo commented Jan 26, 2022

Thank you for the ping!

To me this is an expected consequence of the switch to the ToolsPanel UX pattern (which caused the removal of the Reset buttons), and I appreciate that it may be counter intuitive for users.

Regarding the ToggleGroupControl component — as stated by @aaronrobertshaw, it uses a RadioGroup internally, which means that it's not possible to deselect an option by clicking on it again (I'm also reluctant to change the ToggleGroupControl implementation).

The correct solution for me would be to display the default option as preselected — although it looks like it may be problematic:

which we can't do without adding extra options to the control and also gaining access to merged theme.json and global styles within the block editor.

Another alternative (which I don't really like, since it'd be quite hacky) is to add another option to the ToggleGroupControl called "reset" or "default" which unselects the value — but again, far from ideal.

Otherwise, we could look into refining the ToolsPanel UX further to make it more clear that it can be used to reset values

@ciampo
Copy link
Contributor

ciampo commented Jan 26, 2022

Another thing that came to mind — the font size picker can also display a dropdown instead of the ToggleGroupControl and an input field (in case of custom values), and therefore fixing ToggleGroupControl alone wouldn't probably be enough.

@ntsekouras
Copy link
Contributor

Another thing that came to mind — the font size picker can also display a dropdown instead of the ToggleGroupControl and an input field (in case of custom values), and therefore fixing ToggleGroupControl alone wouldn't probably be enough.

There is another way to 'reset' the other controls - for other inputs we can clear the input value, and in select controls we provide a default option.

(I'm also reluctant to change the ToggleGroupControl implementation).
we could look into refining the ToolsPanel UX further to make it more clear that it can be used to reset values

Having said that, I agree with the above two points about what we should do.

@ramonjd
Copy link
Member

ramonjd commented Jan 27, 2022

The icon was changed to include the plus icon in #34107 with the aim to encourage users to explore that menu for additional options when all optional controls are hidden.

Thanks for the ping and the very neat summary @aaronrobertshaw.

I can confirm this was the original intention: to attempt to highlight that there are more controls available within the Tools Panel dropdown, other than those that are displayed by default.

If the plus sign is causing confusion, especially with the semantic opposition to the minus sign, then maybe that's a "sign" that we should revisit.

Perhaps it would be better for it to only be used in place of the vertical ellipsis when the panel would be completely empty.

Great idea. It's still possible that a block will not declare any default controls in "__experimentalDefaultControls" in its block.json so, barring any bespoke slot fills, the panel would start off empty.

Jan-27-2022.12-06-37.mp4

I will throw up a PR to do this so folks can test drive it.

@paaljoachim
Copy link
Contributor

I too expected to be able to turn off a size go to back to default by clicking the selected state to deselect it, but that does not work. We might meet the same issue again for other controls so hopefully a work around for it can be created that can down the road can be brought onward. I have thought of having a reset button to the left of the Size toggle but it quickly becomes too crowded.

@Clorith
Copy link
Member Author

Clorith commented Feb 7, 2022

For reference, in 5.8, the size toggle was a select box, with the first entry being Default.

@maddisondesigns
Copy link

Having to click the + icon and then click the - icon just to reset the font size is not the least bit intuitive. The older typography tools were easier to use than this new UI. Hiding functionality like this is making it considerably harder for users.

This whole UI change is a step backwards. Having to manually "add" an option like Line Height or Letter Spacing, before you can actually use it, is needlessly confusing and complicated in an already difficult to use editor, and makes editing content even slower. On top of that, you have to add these options on every single block that you want to use them on. They don't even default to being shown on other blocks within the same page, once you've added them to one block. You're making content editing significantly more difficult and considerably slower with these UI changes.

@cbirdsong
Copy link

Echoing that the + button method for reseting any of these is super confusing if you don't know about it and super cumbersome if you do.

The correct solution for me would be to display the default option as preselected

In the case of font sizes, this would either apply the has-medium-font-size class to every paragraph or only apply it to paragraphs that have had the font size changed, even though the control would be visually identical. I don't know the details about these different control types, but it seems clear there needs to be an easy and comprehensible way for a user to empty out any value they've set on something.

In the meantime, an easy way to triage this would just be switching to the <select>-style design as the default instead of only using it when there are lots of font sizes.

@k1sul1
Copy link

k1sul1 commented Mar 25, 2022

Who thought this was a good idea? Literally ANY competetent designer would've gone NO GOD PLEASE NO NOOOOOOOOOO over this bullshit UI.

@paaljoachim
Copy link
Contributor

paaljoachim commented Aug 19, 2022

What about having a preset called inherit/default or something like that. As @glendaviesnz was working on presets PR that was just merged a few days ago. It included an inherit/default preset.
So that we can click the inherit option to go back to what it was originally?

@glendaviesnz
Copy link
Contributor

There is an issue looking at this in the wider context of all the controls here #43082

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests