SiteTitle: Add bold and italic toolbar buttons for convenience by sejas · Pull Request #46489 · WordPress/gutenberg

sejas · 2022-12-13T03:10:19Z

Closes: Site Title Block: Add a toolbar shortcut to italicize the text #45854

What?

Add Font face control to the SiteTitle toolbar where users can use Bold and Italic text styles:

Why?

The SiteTitle style options are difficult to access. I propose adding the bold and italic buttons to the toolbar allowing users to edit the text style easily.
In addition, we improve the accessibility of the block by allowing the shortcuts Ctrl + B to make the text bold and Ctrl + I to make it italic.

How?

I've added two new Toolbar buttons to the SiteTitle block. I'm using the attribtues.style as the source of truth of the bold and style state. For this reason, the new buttons and the Appearance Selector are in sync. Check the screencast to see it in action.

Testing Instructions

Go to Appearance > Editor.
Click on Edit.
Select your SiteTitle block.
Observe that there are a new Typography button on the Toolbar, click it and observe the dropdown with the Bold and Italic options.
Click on the Bold button.
Observe the text transforms to bold.
Click on the Italic button.
Observe the text transforms to italic.
Observe that the Appearance select in the inner settings has changed too. The toolbar and the select are in sync.
Save the template.
Observe the Frontend of the site shows the correct styles.

Testing Instructions for Keyboard

Select the SiteTitle block
Press Ctrl + B or ⌘ + B
Observe the text gets bold.
Press Ctrl + I or ⌘ + I
Observe the text gets italic.

Screenshots or screencast

Before	After

site-title-bold-italic-different-group.mp4

The Typography icon was updated here #46489 (comment)

github-actions · 2022-12-13T03:10:43Z

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @sejas! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

danielbachhuber

Awesome work, @sejas !

Overall, the buttons and keyboard shortcuts seem to work well.

I noticed a couple UX inconsistencies...

If the Site Title is already italicized, then it's not possible to remove the italicization:

When you have some partially selected text, clicking "Bold" makes the entire text bold:

Seen above, if you click "Bold", the inherited italicization is reverted.

The first UX inconsistency isn't new in this pull request, so I don't think we need to address it.

I personally think the second UX inconsistency is acceptable because the toolbar buttons are a UX enhancement for the majority of use cases. But, I wanted to call it out so it's stated.

The third UX inconsistency should probably be addressed.

packages/block-library/src/site-title/edit/use-typography-style.js

sejas · 2022-12-13T17:50:54Z

@danielbachhuber, thanks for testing it and adding your feedback.

If the Site Title is already italicized, then it's not possible to remove the italicization:

Did you add "Additional CSS" to your theme?
The user can add external CSS that modifies the appearance of any block. I think we shouldn't worry about it. If you add the same external CSS rule to a Paragraph, Can you undo the italicization using the toolbar buttons?.

When you have some partially selected text, clicking "Bold" makes the entire text bold:

Yes, that's right. The Bold and Italic buttons add inline CSS to the whole block changing the appearance/style of the entire text. We don't support partial bold or italic as RichText does because we would need to add HTML tags to the site title, changing the semantics and conflicting with the current Appearance selector that already exists.

Seen above, if you click "Bold", the inherited italicization is reverted.

I've tested it using the original SiteTitle, and I have experienced the same behavior. If I add external CSS to the theme, and I choose Bold using the "Appearance selector" the italicization is reverted to regular too.

The buttons behave identically as the "Appearance selector" does. This behavior already exists on SiteTitle, and this PR did not introduce it.
From my point of view, if the user/theme adds external CSS rules, then the behavior of the block can be compromised. Similar to point 1.
If you think it's necessary, I'm happy to revisit it or make a bigger refactor on the SiteTitle block.

danielbachhuber · 2022-12-13T20:02:01Z

Did you add "Additional CSS" to your theme? The user can add external CSS that modifies the appearance of any block. I think we shouldn't worry about it.

@sejas I didn't, no. I agree that we don't need to worry about it.

The buttons behave identically as the "Appearance selector" does. This behavior already exists on SiteTitle, and this PR did not introduce it.
From my point of view, if the user/theme adds external CSS rules, then the behavior of the block can be compromised. Similar to point 1.

Ah, ok. That's fine, then.

aaronrobertshaw

Excellent work on this @sejas 👍

✅ Everything tests as advertised for me.

The generally low prominence of the font appearance control was by design. There’s some history in #35604, but given the direction on the original issue, I'd say the block toolbar additions and current behaviour are appropriate.

I don’t see any of the floated UX issues as blockers for this PR. They are potential opportunities to iterate further on the typography block supports in follow-ups.

If the Site Title is already italicized, then it's not possible to remove the italicization:

The italic style should be able to be removed by selecting the "Regular" option in the font appearance control within the block inspector's typography panel.

Do other blocks hard-code the bold value in this way?

Given how tightly coupled a lot of this is to the typography block support, I wonder if it would be better to move some of this, such as the useTypographyStyles hook, to the typography block support and consume it from there.

It might also avoid duplicating/syncing the bold value while allowing other blocks to potentially adopt the same feature as has been added to the Site Title block in this PR.

While exploring that option, perhaps it might be helpful to rename the hook to something more reflective of the subset of typography styles it is returning, e.g. useFontAppearanceStyles.

packages/block-library/src/site-title/edit/use-typography-style.js

sejas · 2022-12-14T19:48:08Z

@aaronrobertshaw, It's very smart to move the hook to the typography block support and share the bold/italic values to avoid future differences 👍 . Thanks for the suggestion 🙌.

I'm considering adding new keys to the supports.typography to show the new toolbar buttons. Other core blocks could use them just by enabling them. If we go this way, I will need to investigate how to add the shortcuts.

For example:

"supports": {
   ...
   "typography": {
	"fontSize": true,
	"lineHeight": true,
	"__experimentalFontFamily": true,
	"__experimentalTextTransform": true,
	"__experimentalTextDecoration": true,
	"__experimentalFontStyle": true,
	"__experimentalFontWeight": true,
	"__experimentalLetterSpacing": true,
	"__experimentalDefaultControls": {
		"fontSize": true,
		"lineHeight": true,
		"fontAppearance": true,
+		"fontAppearanceToolbar": true, // <- NEW. It would also check if `__experimentalFontStyle` or/and `__experimentalFontWeight` are enabled.
		"letterSpacing": true,
		"textTransform": true
	}
}

aaronrobertshaw · 2022-12-14T23:43:23Z

I'm considering adding new keys to the supports.typography to show the new toolbar buttons. Other core blocks could use them just by enabling them. If we go this way, I will need to investigate how to add the shortcuts.

That sounds like a logical next step to me 👍

Might I suggest, though, that you move that exploration to a follow-up PR?

That way, the great work already done in this PR can land sooner and won't get bogged down by discussions around how best to fit the proposed change into the shape of the block supports, what blocks could or should benefit from it etc.

sejas · 2022-12-15T17:58:03Z

Might I suggest, though, that you move that exploration to a follow-up PR?

I agree. Let's finish this PR first 👌.

I've updated this PR with the following changes:

Renamed the hook to useFontAppearanceStyles and moved to block-editor/src/hooks. I've decided to create a separate file. Another good place could be inside font-appearance.js.
I'm consuming the hook with the prefix __experimental.
font-appearance-control and the new hook use the same variables for Bold and Italic.
Moved the keydown/shotrcut logic inside the hook.

I still need to change the icons to differentiate them from the RichText buttons. I'll post some design suggestions in the issue: #45854 (comment)

aaronrobertshaw

Thank you for the continued iterations on this PR @sejas 👍

It is still testing well for me.

The one thing I'm not certain about is the introduction of the __experimental API. I'm aware that there's a concerted effort to avoid introducing experimental APIs as they previously had a habit of working their way into core.

Recently, a new approach to managing experiments was merged via #43386. I get the feeling that might be overkill for this particular case, though.

Perhaps we could get additional reviews and input on this PR to avoid the experimental stage for the new custom hook altogether. This might also be able to occur while we're waiting on the new bold and italic icons for the toolbar buttons.

What do you think?

packages/block-library/src/site-title/edit/index.js

packages/block-editor/src/hooks/use-font-appearance-styles.js

packages/block-library/src/site-title/edit/index.js

sejas · 2022-12-20T10:15:52Z

Perhaps we could get additional reviews and input on this PR to avoid the experimental stage for the new custom hook altogether. This might also be able to occur while we're waiting on the new bold and italic icons for the toolbar buttons.

What do you think?

Yes, I think it would be a great idea to avoid the experimental stage for the new hook 👌

kozer · 2022-12-20T13:31:56Z

@sejas very nice work!!!

Everything looks nice. Really loved the constants that you used for bold, italic etc..

As @danielbachhuber , I have the same UI inconsistencies:

If the Site Title is already italicized, then it's not possible to remove the italicization
When you have some partially selected text, clicking "Bold" makes the entire text bold (This is not happening in the Custom link block for example)
If you click "Bold", the inherited italicization is reverted.

I also noticed that by changing the style back to "Regular", the italics are being removed. and pressing the button again, we end up in the 1 case.

Again, nice work!

…rdpress/block-editor

…arance

jameskoster · 2023-02-24T12:28:35Z

Thanks for the update @sejas, are you using the typography icon from the WordPress Icons package? It looks a bit different.

sejas · 2023-02-24T12:40:21Z

Thanks for the update @sejas, are you using the typography icon from the WordPress Icons package? It looks a bit different.

Oh!, I used formatUppercase icon.
Fixed here fa2b28b 👌.
@jameskoster, Thanks for catching the mistake.

jameskoster · 2023-02-24T13:29:10Z

Thanks for changing that detail :)

The icon isn't the perfect one, but I'm struggling to think of a better one right now. For me it's probably okay to continue moving forwards with this one, and circle back to explore a new icon later. But I'd appreciate another set of eyes from @WordPress/gutenberg-design in case I'm missing an obvious option.

jasmussen · 2023-02-27T10:27:16Z

Happy to try this and revisit.

jameskoster · 2023-02-27T11:55:28Z

I just took this for a spin:

Bold doesn't appear to be working, could that be something on my local setup?

I'm not sure we need to the 'toggled' state on the toolbar button when styles are selected.

sejas · 2023-02-27T12:34:16Z

@jameskoster

Bold doesn't appear to be working, could that be something on my local setup?

Does the Bold style work for you when using the original "Typography panel" ?

I'm not sure we need to the 'toggled' state on the toolbar button when styles are selected.

Happy to remove it if needed. I intended to make clear that some state is applied inside. The heading and paragraph buttons use the Value Icon to represent this state. But we can't use the B I icons, so I went for the toggled versions with black background.

jameskoster · 2023-02-27T17:09:12Z

Does the Bold style work for you when using the original "Typography panel" ?

No. I guess that's an issue on my local :)

Happy to remove it if needed

Yeah I don't think it's really necessary.

danielbachhuber

Works well to my testing!

CleanShot.2023-02-27.at.14.16.14.mp4

FYI - I merged trunk.

Probably outside the scope of this PR, but it might be cool if the icon reflected the font appearance (e.g. appeared as bolded when "Bold" was selected, italicized when "Italic" was selected, etc.)

I'll defer to the @WordPress/gutenberg-core folks for final approval on the code.

youknowriad · 2023-02-28T08:16:35Z

packages/block-library/src/site-title/edit/index.js

 	InspectorControls,
 	BlockControls,
 	useBlockProps,
+	__experimentalUseFontAppearanceStyles as useFontAppearanceStyles,


Any reason for this to leave in the block editor if it's only used in the site-title block?

Maybe folks working on block tools can shed more light on this new toolbar. cc @aaronrobertshaw @andrewserong @jasmussen

Ok, just saw that @aaronrobertshaw is already reviewing this PR :)

👍 Re-reviewing this PR is on my radar for the next couple of days.

The logic behind the hook living in the block-editor package was that it is so closely tied to the typography block supports in that package. There's also the potential for it to be re-used across other blocks leveraging the font weight and style supports.

Maybe we can start with it being "private" unless we're certain that it's an API we want to offer to third-party devs.

aaronrobertshaw

Thanks for continuing to iterate on this one @sejas 👍

In its current state, the PR is testing very nicely for me. Great work!

Before landing this, there are a couple more things we should probably address.

Removing the toggled state for the block toolbar's typography dropdown.
Now we have the capability to lock and unlock experimental APIs, I think the best and quickest path to landing this will be to leverage that as @youknowriad suggested. You can find more info on this in the coding guidelines.
(Optional) If the intent is for more than just the Site Title block to eventually offer the block toolbar control for font appearance, moving the new FontAppearanceControl to a component alongside the hook so it can be reused might make sense. If doing this, it should be locked as private/experimental as well.

With the above tweaks addressed, I think this PR will be in good shape. ✨

sejas · 2024-10-02T11:47:56Z

Closing this PR, since I won't focus on it in the short time. Thank y'all!

sejas requested a review from ajitbohra as a code owner December 13, 2022 03:10

github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Dec 13, 2022

sejas mentioned this pull request Dec 13, 2022

Site Title Block: Add a toolbar shortcut to italicize the text #45854

Open

danielbachhuber self-requested a review December 13, 2022 12:31

danielbachhuber reviewed Dec 13, 2022

View reviewed changes

packages/block-library/src/site-title/edit/use-typography-style.js Outdated Show resolved Hide resolved

danielbachhuber assigned sejas Dec 13, 2022

danielbachhuber changed the title ~~SiteTitle: Add bold and italic controls~~ SiteTitle: Add bold and italic toolbar buttons for convenience Dec 13, 2022

danielbachhuber self-requested a review December 13, 2022 20:02

danielbachhuber approved these changes Dec 13, 2022

View reviewed changes

danielbachhuber added the [Block] Site Title Affects the Site Title Block label Dec 13, 2022

danielbachhuber requested review from aaronrobertshaw, draganescu and glendaviesnz December 13, 2022 22:36

aaronrobertshaw reviewed Dec 14, 2022

View reviewed changes

packages/block-library/src/site-title/edit/use-typography-style.js Outdated Show resolved Hide resolved

sejas requested a review from ellatrix as a code owner December 15, 2022 17:13

aaronrobertshaw reviewed Dec 16, 2022

View reviewed changes

kozer approved these changes Dec 20, 2022

View reviewed changes

sejas added 5 commits February 20, 2023 18:07

SiteTitle: add buttons to change the style appearance

d24d0ff

SiteTitle: handle default state for not bold and not italic

e6e886d

SiteTitle: abstract onClick bold and italic

9345443

SiteTitle: create Bold and ItalicControl components

73c83f4

SiteTitle: add keyboard shortcuts and remove controls

7631f42

sejas added 10 commits February 20, 2023 18:07

SiteTitle: import __experimentalUseFontAppearanceStyles hook from @wo…

04d409e

…rdpress/block-editor

SiteTitle: move shortcuts logic into the hook onKeyDownToggleFontAppe…

20285fb

…arance

SiteTitle: remove test code

919af71

SiteTitle: remove useTypographyStyle in favor of useFontAppearanceStyles

bdc5328

SiteTitle: declare onKeyDownToggleFontAppearance as a function

0332400

SiteTitle: import hook without experimental prefix

b3aa8bc

SiteTitle: rename font appearance function

58e0448

SiteTitle: refactor font appearance inside a toolbar dropdown menu

e536a4c

SiteTitle: use isActive instead of isSelected control attribute

f17bfaa

SiteTitle: add isPressed icon status to font appearance control

dcddcf9

sejas force-pushed the add/site-title-bold-italic-styles branch from 27dfa75 to dcddcf9 Compare February 20, 2023 18:29

sejas added 2 commits February 20, 2023 18:35

SiteTitle: move FontAppearanceControl to other toolbar group

0dfc1f2

SiteTitle: fix typo on appearance

95ed3ee

SiteTitle: update typography icon

fa2b28b

Merge branch 'trunk' into add/site-title-bold-italic-styles

9ef92e1

danielbachhuber requested review from aaronrobertshaw and danielbachhuber February 27, 2023 22:12

danielbachhuber approved these changes Feb 27, 2023

View reviewed changes

youknowriad reviewed Feb 28, 2023

View reviewed changes

aaronrobertshaw reviewed Mar 1, 2023

View reviewed changes

sejas closed this Oct 2, 2024

sejas deleted the add/site-title-bold-italic-styles branch October 2, 2024 11:48

Conversation

sejas commented Dec 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What?

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

Uh oh!

github-actions bot commented Dec 13, 2022

Uh oh!

danielbachhuber left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sejas commented Dec 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danielbachhuber commented Dec 13, 2022

Uh oh!

aaronrobertshaw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sejas commented Dec 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aaronrobertshaw commented Dec 14, 2022

Uh oh!

sejas commented Dec 15, 2022

Uh oh!

aaronrobertshaw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sejas commented Dec 20, 2022

Uh oh!

kozer commented Dec 20, 2022

Uh oh!

jameskoster commented Feb 24, 2023

Uh oh!

sejas commented Feb 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jameskoster commented Feb 24, 2023

Uh oh!

jasmussen commented Feb 27, 2023

Uh oh!

jameskoster commented Feb 27, 2023

Uh oh!

sejas commented Feb 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jameskoster commented Feb 27, 2023

Uh oh!

danielbachhuber left a comment

Choose a reason for hiding this comment

Uh oh!

youknowriad Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

youknowriad Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

aaronrobertshaw Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

youknowriad Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

aaronrobertshaw left a comment

Choose a reason for hiding this comment

Uh oh!

sejas commented Oct 2, 2024

Uh oh!

Reviewers

sejas commented Dec 13, 2022 •

edited

Loading

sejas commented Dec 13, 2022 •

edited

Loading

sejas commented Dec 14, 2022 •

edited

Loading

sejas commented Feb 24, 2023 •

edited

Loading

sejas commented Feb 27, 2023 •

edited

Loading