Block Library: Allow changing Site Title block heading level. #20361
Conversation
epiqueras
added
[Type] Enhancement
epiqueras
requested review from
ajitbohra,
jorgefilipecosta,
nerrad,
Soean,
talldan and
youknowriad
as code owners
|
Size Change: +1.27 kB (0%) Total Size: 865 kB
ℹ️ View Unchanged
|
|
@epiqueras This is great!
I am curious if it makes sense to add a Example: Thoughts? |
|
One issue I just noticed as well, is that while changing the setting in wp-admin works, it does not change the output on the frontend, as it still uses |
I am not sure; I think this block needs a proper full design with more features like colors and alignments.
Are you sure you rebuilt things? I see it changing as expected. |
|
@epiqueras I did double check that yeah. Everything has rebuilt properly, and I am seeing the js changes from this PR in action. I think the issue might be that I have the Site Title block in both my header.html file, and in the body of a post on the same page. I added some debugging to the output function, and the first time the attributes are an empty array, and the second time it has level 3, though both are still using
|
|
Its kind of weird to have the Paragraph option in the list. I expected that I'd get all the features of the normal Paragraph block (bolding and the like), but it doesn't do that. |
epiqueras
force-pushed
the
add/site-title-block-features
branch
from
70d9ecb
to
04584fe
Compare
|
@johnstonphilip Short-circuiting wasn't working right; it's fixed now.
That would be a block transformation. Where would you put it instead? |
|
@epiqueras Working now! 👍 |
|
Thanks for testing! |
|
@epiqueras I think you mentioned this but I just want to confirm: Based on my testing, this also appears to fix #20337 Is that right? |
|
Yes, it does. |
Sorry, I'm not suggesting a transformation. I think my hesitation is that this menu is essentially changing the That said, if we want to keep the paragraph option, I think it should be the last in the list. Its both visually the smallest, and semantically less important.
|
|
Is there any connection with this PR? As the same UI would apply to both. |
|
No
|
| 4: 'M9 15H7v-4H3v4H1V5h2v4h4V5h2v10zm10-2h-1v2h-2v-2h-5v-2l4-6h3v6h1v2zm-3-2V7l-2.8 4H16z', | ||
| 5: 'M12.1 12.2c.4.3.7.5 1.1.7.4.2.9.3 1.3.3.5 0 1-.1 1.4-.4.4-.3.6-.7.6-1.1 0-.4-.2-.9-.6-1.1-.4-.3-.9-.4-1.4-.4H14c-.1 0-.3 0-.4.1l-.4.1-.5.2-1-.6.3-5h6.4v1.9h-4.3L14 8.8c.2-.1.5-.1.7-.2.2 0 .5-.1.7-.1.5 0 .9.1 1.4.2.4.1.8.3 1.1.6.3.2.6.6.8.9.2.4.3.9.3 1.4 0 .5-.1 1-.3 1.4-.2.4-.5.8-.9 1.1-.4.3-.8.5-1.3.7-.5.2-1 .3-1.5.3-.8 0-1.6-.1-2.3-.4-.6-.2-1.1-.6-1.6-1-.1-.1 1-1.5 1-1.5zM9 15H7v-4H3v4H1V5h2v4h4V5h2v10z', | ||
| 6: 'M9 15H7v-4H3v4H1V5h2v4h4V5h2v10zm8.6-7.5c-.2-.2-.5-.4-.8-.5-.6-.2-1.3-.2-1.9 0-.3.1-.6.3-.8.5l-.6.9c-.2.5-.2.9-.2 1.4.4-.3.8-.6 1.2-.8.4-.2.8-.3 1.3-.3.4 0 .8 0 1.2.2.4.1.7.3 1 .6.3.3.5.6.7.9.2.4.3.8.3 1.3s-.1.9-.3 1.4c-.2.4-.5.7-.8 1-.4.3-.8.5-1.2.6-1 .3-2 .3-3 0-.5-.2-1-.5-1.4-.9-.4-.4-.8-.9-1-1.5-.2-.6-.3-1.3-.3-2.1s.1-1.6.4-2.3c.2-.6.6-1.2 1-1.6.4-.4.9-.7 1.4-.9.6-.3 1.1-.4 1.7-.4.7 0 1.4.1 2 .3.5.2 1 .5 1.4.8 0 .1-1.3 1.4-1.3 1.4zm-2.4 5.8c.2 0 .4 0 .6-.1.2 0 .4-.1.5-.2.1-.1.3-.3.4-.5.1-.2.1-.5.1-.7 0-.4-.1-.8-.4-1.1-.3-.2-.7-.3-1.1-.3-.3 0-.7.1-1 .2-.4.2-.7.4-1 .7 0 .3.1.7.3 1 .1.2.3.4.4.6.2.1.3.3.5.3.2.1.5.2.7.1z', | ||
| }; |
There was a problem hiding this comment.
Would we save memory if we moved this const out of the function (and into module level) so it's not re-initialized each time the function is called?
There was a problem hiding this comment.
Probably not a relevant amount.
There was a problem hiding this comment.
Looking good overall!
I agree with Shaun that if we're going to keep the option to format the site title as a paragraph, the paragraph option should be last in the format picker (even if that makes looping over 0..6 harder).
Another (more architectural yet vague) question: I seem to recall that there were plans to support composing blocks from others, with the composed (outermost) block controlling the composing blocks' attributes, and having the composed block 'inherit' the toolbar (and inspector?) controls from the composing ones. (Unfortunately, I can't seem to find any references right now.) Would it thus be possible to compose the site title block from a heading block?
That's more for blocks that have a hierarchy, e.g., nav menu -> nav link. |
Not sure we're on the same page here, I don't mean the established Inner Blocks pattern, but a pattern that I thought has been emerging more recently -- with a more rigid connection between the 'outer' (composed) and 'inner' (composing) blocks. I thought I'd seen something to that effect, but maybe I'm mistaken 🤔 |
Ahh, I think I was mostly thinking of your own context concepts from #19685 and #19572 😊 |
|
That still is for hierarchical blocks. The only block I can think of that absorbs the toolbar, as you describe, is the Navigation block. |
Closes #20333
Description
This PR makes the heading level of the Site Title block configurable through the block toolbar. It allows you to set it to all six heading levels or a paragraph level.
It also fixes a bug in
core-datawhere a null value was having properties accessed.How has this been tested?
It was verified that changing the heading level works as expected in the editor and the front end.
Types of Changes
New Feature: The Site Title block's heading level can now be configured.
Checklist: