-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Post title: Re-add the paragraph level (without UI) #60548
Conversation
Size Change: +7 B (0%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I would like to temporarily pause the merging of this PR.
This is because supporting the paragraph level in the Site Title block seems to have accessibility concerns, as discussed in #30549.
Regarding support for the paragraph, the following specification changes have been made.
- Up to WP6.2, level zero (paragraph) was supported internally.
- In WP6.3, level zero is no longer considered, so incorrect
h0
tag is output
If we re-add paragraph block support now, we might need to remove this support again in the future if paragraph support is deemed inappropriate from an accessibility perspective. In fact, #42590, which is similar to this PR, has been closed.
I think it would be better to discuss this issue further to see if there is a more ideal solution.
@alexstine Please let me know if you have any feedback regarding this PR 🙏
Thanks for the extra context @t-hamano, I didn't realise there was so much prior discussion surrounding it! Apologies, I should have read a bit more of the history before hitting ✅. Good idea holding off for now and getting more feedback 👍 |
@t-hamano I really don't mind if this is allowed internally that way our blocks can take advantage of it. What I don't want is this to be put in reach of end users. For example, this is a very valid use-case.
This is not.
I am very against shipping a feature to core that will allow people to output titles in a very semantically bad format for accessibility. Because one could style these links to look like a list visually with no list semantics being communicated to end users of assistive tech. Because it only uses the paragraph tag over a heading tag, it makes it even more undiscoverable. The only method would be the link itself which we all know how reliable that is, sarcasm noted. My answer is still no for the UI but yes if it is needed internally. CC: @afercia @joedolson Thanks. |
Yes it is because of the accessibility concerns that I did not include it when the HeadingLevelDropdown was moved to a component. Many use cases for linking to the current post could be solved if the Link Controls supported it. |
Whether a title with a paragraph is discoverable or useful is going to depend on the layout. |
I pushed the code changes that removes the paragraph from the drop down, but I do not have time to update the PR description right now. With this change, |
The use case mentioned in this comment, which is about the ability to use the Post title in a Query loop for a simple list of posts, makes perfectly sense to me. A heading would not be appropriate in that case. However, I don't think the ability to render a heading as a paragraph should be in the heading level dropdown menu. Conceptually and semantically, the heading level dropdown menu is specific to the concept of 'level'. A 'level' is only for headings. The toggle button that opens the menu is labeled 'Change level'. The menu itself is labeled 'Change level'. If I remember correctly this was one of the arguments to remove the It's a rendering option. As such, I'd tend to think it should go in the block variations. That wouldn't be much different from what other blocks already do. For example the Group block provides both block transforms and block varaitions. The Group block can be transformed to:
I'd consider to add the HTML tag choice to the variations. It would be useful for other blocks too. I'd also agree that it would be nice to explore an option fo render the Post title as 'Plain text', ie.: with no wrapping HTML tag. I'd also like to remind that, for exampke. the Group block does provide a setting to choose the HTML tag the block is rendered with. It's a little buried down in the block Settings > Advanced section and it should probably be more prominent. I'd would like to see this HTML element setting moved to the block variations. Screenshot of the Group block HTML element setting: I will create a new issue for discussion. |
@alexstine @afercia, thank you for your comments. Based on your opinion, is it okay to support paragraph level internally? In other words, we will ship this PR as is. The Post Title block supported heading level in #22872 about 4 years ago. At the same time, it seems that the paragraph level is also supported internally (I don't know why the paragraph was was supported). However, given this fact, I feel that allowing the paragraph level internally makes sense from a backward compatibility perspective.
I remembered the Block Binding API. Looking at the video attached to this comment, it appears that binding the post title to any block via the UI is being considered. If this could be applied, I think it would be possible to express the post title text in a more plain format. |
We also can't assume that there will be and not having a heading is worse than a possible duplicate. Thanks for the updated PR. |
I don't think the paragraph 'level' should be implemented internally as a 'level' becasue, well, it's not a level. ReL the transforms: I didn't mean to use block transforms. I suggested to consider block variations transforms, which is a different thing. To me, it seems that from an user perspective a few concepts are pretty confusing in the UI:
All this is extremely confusing and I'm not sure users are interested to know whether it's a block transform or a block varaition or a HTML element setting. The just want to change how the block renders. Overall. I'd think all these transforms / variations / HTML element settings should be reconsidered and made way ore intuitive for users. |
I disagree with the suggestion to move the level selection from the toolbar to the element selection in the Advanced panel.
The HTML element selection is placed in the Advanced panel because it is an advanced tool that requires some knowledge of HTML and because it is an action that isn't used that often. Editing the text and adding appropriate headings is a more frequent action than changing a div to a span. Using Google Docs as an example, you can select "regular text", (not "paragraph") and it is in the same option as the heading selection. (it might be "normal text", I am not sure because my Drive is in Swedish so I am translating). Using Microsoft Word as an example, you select "normal" in the same option that you select headings. But in that option you can also select strong, quote, title, subtitle, list etc. And they are deliberately calling it a style option. |
The component name |
@t-hamano As far as I'm aware, there is not a setting in the classic editor to make a post title a paragraph. I don't honestly care if there is a paragraph option for some blocks, just not like this. If a query block wants to display the post titles as paragraphs because it is using a semantic list, great. I will never support giving users the ability to choose this from the UI. This is a feature that we should support for developers, nothing more, nothing less. Otherwise, this is going to be prone to terrible misuse. |
I already removed the option from the UI yesterday. The PR only fixes the regression where the paragraphed used to work when it was added manually in the code. There is one UI change. When a developer or advanced user has manually changed the code to use a paragraph, the toolbar will show the paragraph icon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@carolinan Thanks for clearing that up.
@t-hamano I think we're good to unblock this now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's ship this PR.
As mentioned in this comment, whether it was intentional or not, it's true that we've been allowing the paragraph level internally for nearly four years. This PR fixes a regression that occurred in WP6.3, and I believe paragraph levels should be allowed, at least internally, to maintain backward compatibility.
Let's continue discussing overall UI issues and improvements in a separate issue.
What?
Enables using post titles as paragraphs.
Closes #53594
Why?
Previously, using a post title as a paragraph was possible by changing the markup. This ability was removed in 6.3.
This PR allows using paragraphs again, but only by editing the block markup and setting the "level" block attribute to 0.
How?
The PR
<p>
if the level attribute is set to 0.Testing Instructions
Apply the PR.
Either copy and paste this code snippet into the block editor, or add a post title block and manually edit the level attribute in the code editor mode.
<!-- wp:post-title {"level":0} /-->
<p>
.Screenshot
Below is a screenshot of the post title block in the block editor. The only UI change is that the toolbar shows the paragraph icon when someone has set the level attribute to 0.