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

Gutenberg Plugin: Add hook to allow writing-mode as a safe CSS property #54581

Merged
merged 1 commit into from
Sep 19, 2023

Conversation

t-hamano
Copy link
Contributor

@t-hamano t-hamano commented Sep 19, 2023

Fixes #54344
Alternative to #54345
For more information, see this comment

What?

This PR includes the following three:

  • Added safe_style_css filter to prevent wiriting-mode property from being removed by safecss_filter_attr() function
  • Remove explicit addition of writing-mode style in render callback function of the Post Navigation Link block which is no longer needed
  • Updated the Style Engine testing

Why?

For dynamic blocks, as with other block support styles, there is no need to explicitly pass the style, as it should be automatically output by the get_wrapper_attributes() function.

However, this property is not yet allowed in the core, so we need to allow it in the Gutenberg plugin first.

How?

writing-mode (text orientation) is supported in WordPress 6.3. Therefore, this hook originally needed to be added in the lib/compat/wordpress-6.3 directory. However, this property will only be allowed in core WordPress 6.4 and above, so I added the hook to the lib/compat/wordpress-6.4 directory.

Testing Instructions

To test this PR, enable Emptytheme and update theme.json as below:

{
	"$schema": "https://schemas.wp.org/trunk/theme.json",
	"version": 2,
	"settings": {
		"appearanceTools": true,
		"layout": {
			"contentSize": "840px",
			"wideSize": "1100px"
		},
		"typography": {
			"writingMode": true
		}
	}
}

Footnotes Block

  • Add some footnotes to the paragraph block.
  • In the Typography panel of the Footnotes block, change orientation to "vertical".
  • In the front end, the block should be displayed vertically.

Post Navigation Block

  • Create three posts.
  • Insert a Previous Post block and a Next Post block in the middle post.
  • In the Typography panel, change orientation to "vertical".
  • In the front end, the block should be displayed vertically.

@t-hamano t-hamano added [Type] Bug An existing feature does not function as intended Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts labels Sep 19, 2023
@github-actions
Copy link

This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress.

If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged.

If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack.

Thank you! ❤️

View changed files
❔ lib/compat/wordpress-6.4/kses.php
❔ lib/load.php
❔ phpunit/style-engine/style-engine-test.php

@t-hamano t-hamano added the Needs PHP backport Needs PHP backport to Core label Sep 19, 2023
@t-hamano t-hamano marked this pull request as ready for review September 19, 2023 03:01
@t-hamano t-hamano self-assigned this Sep 19, 2023
@carolinan
Copy link
Contributor

As far as I know, it was never supported in 6.3. I don't know if this was a decision made or if the backport just did not get done.

@carolinan
Copy link
Contributor

Another question is if we want to add support for text-orientation at the same time or if this needs to wait until later:
#52624

@t-hamano
Copy link
Contributor Author

As far as I know, it was never supported in 6.3. I don't know if this was a decision made or if the backport just did not get done.

Sorry, this was my mistake. Indeed, writing mode seems to be supported starting with WordPress 6.4.

https://core.trac.wordpress.org/ticket/59306

Another question is if we want to add support for text-orientation at the same time or if this needs to wait until later:

Personally, I think it would be better to merge this PR first, which has fewer files to add/update. That way, #52624 would only need to update the safe_style_css hook and style engine test after rebasing with the latest trunk.

Comment on lines -31 to 35
$styles = '';
if ( isset( $attributes['style']['typography']['writingMode'] ) ) {
$styles = "writing-mode: {$attributes['style']['typography']['writingMode']};";
}
$wrapper_attributes = get_block_wrapper_attributes(
array(
'class' => $classes,
'style' => $styles,
)
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this change will be bundled in with the packages update for core, would it be worth trying to land the change to core's safe_style_css list before attempting to land this Gutenberg PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, I think that's certainly true.

Would it be ideal to wait to merge this PR until a patch has been submitted to the core that adds writing-mode to the safe_style_css list and is committed in the core?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it necessarily needs to be committed to core before merging this PR, but I'd recommend opening a trac ticket and corresponding PR for wordpress-develop before merging if you have the time, that way the change will be ready to land in time.

Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is working nicely for me, thanks for adding it in!

✅ Writing-mode now outputs correctly for the footnotes block on the site frontend
✅ With this change the writing mode still outputs correctly for the post navigation link block

image

Just left a comment about backporting to core. If this PR lands for 6.4, it'd be good to ensure the backport for core lands as well so that the adjustment to the post navigation link block doesn't cause any regressions.

LGTM! ✨

@t-hamano
Copy link
Contributor Author

Added a ticket and a patch to the core: https://core.trac.wordpress.org/ticket/59387

Also added to the backport tracking issue #54177.

I think everything is ready now, so let's merge this PR 👍

@t-hamano t-hamano merged commit 2cfe1ba into trunk Sep 19, 2023
59 checks passed
@t-hamano t-hamano deleted the fix/writing-mode-kses branch September 19, 2023 05:49
@github-actions github-actions bot added this to the Gutenberg 16.7 milestone Sep 19, 2023
@t-hamano
Copy link
Contributor Author

@carolinan Could you rebase #52624 with the latest trunk and add text-orientation to the hook in lib/compat/wordpress-6.4/kses.php?

pento pushed a commit to WordPress/wordpress-develop that referenced this pull request Sep 19, 2023
Original PR from Gutenberg repository:
* [WordPress/gutenberg#54581 #54581 Gutenberg Plugin: Add hook to allow `writing-mode` as a safe CSS property]

Reference: [https://developer.mozilla.org/en-US/docs/Web/CSS/writing-mode MDN Web Docs: writing-mode].

Follow-up to [56605].

Props wildworks, mukesh27, poena, andrewserong.
Fixes #59387.

git-svn-id: https://develop.svn.wordpress.org/trunk@56617 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Sep 19, 2023
Original PR from Gutenberg repository:
* [WordPress/gutenberg#54581 #54581 Gutenberg Plugin: Add hook to allow `writing-mode` as a safe CSS property]

Reference: [https://developer.mozilla.org/en-US/docs/Web/CSS/writing-mode MDN Web Docs: writing-mode].

Follow-up to [56605].

Props wildworks, mukesh27, poena, andrewserong.
Fixes #59387.
Built from https://develop.svn.wordpress.org/trunk@56617


git-svn-id: http://core.svn.wordpress.org/trunk@56129 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-actions bot pushed a commit to platformsh/wordpress-performance that referenced this pull request Sep 19, 2023
Original PR from Gutenberg repository:
* [WordPress/gutenberg#54581 #54581 Gutenberg Plugin: Add hook to allow `writing-mode` as a safe CSS property]

Reference: [https://developer.mozilla.org/en-US/docs/Web/CSS/writing-mode MDN Web Docs: writing-mode].

Follow-up to [56605].

Props wildworks, mukesh27, poena, andrewserong.
Fixes #59387.
Built from https://develop.svn.wordpress.org/trunk@56617


git-svn-id: https://core.svn.wordpress.org/trunk@56129 1a063a9b-81f0-0310-95a4-ce76da25c4cd
pereirinha pushed a commit to pereirinha/wordpress-develop that referenced this pull request Sep 21, 2023
Original PR from Gutenberg repository:
* [WordPress/gutenberg#54581 #54581 Gutenberg Plugin: Add hook to allow `writing-mode` as a safe CSS property]

Reference: [https://developer.mozilla.org/en-US/docs/Web/CSS/writing-mode MDN Web Docs: writing-mode].

Follow-up to [56605].

Props wildworks, mukesh27, poena, andrewserong.
Fixes #59387.

git-svn-id: https://develop.svn.wordpress.org/trunk@56617 602fd350-edb4-49c9-b593-d223f7449a82
@SiobhyB SiobhyB removed the Needs PHP backport Needs PHP backport to Core label Sep 25, 2023
@carolinan
Copy link
Contributor

@carolinan Could you rebase #52624 with the latest trunk and add text-orientation to the hook in lib/compat/wordpress-6.4/kses.php?

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Footnotes: writingMode (text orientation) is not visible on the front
4 participants