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

Multiple blocks: Add missing has-link-color class to the front #47357

Merged
merged 12 commits into from
Jan 25, 2023

Conversation

carolinan
Copy link
Contributor

@carolinan carolinan commented Jan 23, 2023

What?

Adds the has-link-color class to the front of several dynamic blocks when a link color is chosen.
Closes #47233

Why?

The class is present in the editor but not on the front.

This is an edge case:

  • Twenty Twenty-One needs to use this class to determine wether or not it should change the link color depending on the background color.
  • Users will only be able to change the link color in Twenty Twenty-One if the appearance-tools theme support is enabled, which is suggested -not yet committed -in https://core.trac.wordpress.org/ticket/56487

Why these blocks? Because they showed up as problematic when testing 56487. There may be more dynamic blocks affected that have not been tested yet.

How?

This PR manually adds the has-link-color class through get_block_wrapper_attributes in index.php.

Testing Instructions

You can test this in any block theme with support for link colors.

  1. First you need to enable comment pagination in the admin area under Settings > Discussion.
  2. Then you need a post or page that has comments. One of the comments need to have a link in the content. Open this content in the block editor.
  3. Add a comments block and add link colors to the inner blocks, or use this sample code:
<!-- wp:comments -->
<div class="wp-block-comments"><!-- wp:comments-title /-->

<!-- wp:comment-template -->
<!-- wp:columns -->
<div class="wp-block-columns"><!-- wp:column {"width":"40px"} -->
<div class="wp-block-column" style="flex-basis:40px"><!-- wp:avatar {"size":40,"style":{"border":{"radius":"20px"}}} /--></div>
<!-- /wp:column -->

<!-- wp:column -->
<div class="wp-block-column"><!-- wp:comment-author-name {"style":{"elements":{"link":{"color":{"text":"var:preset|color|vivid-cyan-blue"}}}},"fontSize":"small"} /-->

<!-- wp:group {"style":{"spacing":{"margin":{"top":"0px","bottom":"0px"}}},"layout":{"type":"flex"}} -->
<div class="wp-block-group" style="margin-top:0px;margin-bottom:0px"><!-- wp:comment-date {"style":{"elements":{"link":{"color":{"text":"var:preset|color|luminous-vivid-amber"}}}},"fontSize":"small"} /-->

<!-- wp:comment-edit-link {"style":{"elements":{"link":{"color":{"text":"var:preset|color|vivid-green-cyan"}}}},"fontSize":"small"} /--></div>
<!-- /wp:group -->

<!-- wp:comment-content {"style":{"elements":{"link":{"color":{"text":"var:preset|color|vivid-purple"}}}}} /-->

<!-- wp:comment-reply-link {"style":{"elements":{"link":{"color":{"text":"var:preset|color|vivid-purple"}}}},"fontSize":"small"} /--></div>
<!-- /wp:column --></div>
<!-- /wp:columns -->
<!-- /wp:comment-template -->

<!-- wp:comments-pagination {"style":{"elements":{"link":{"color":{"text":"#00bedc"}}}}} -->
<!-- wp:comments-pagination-previous /-->

<!-- wp:comments-pagination-numbers /-->

<!-- wp:comments-pagination-next /-->
<!-- /wp:comments-pagination -->

<!-- wp:post-comments-form {"style":{"elements":{"link":{"color":{"text":"var:preset|color|luminous-vivid-orange"}}}}} /--></div>
<!-- /wp:comments -->
  1. Save and view the front. Confirm that the link color is still working and that the class name has-link-color is now added to the wrappers.

The post comments link can only be added in the site editor.
Open the site editor and add a query loop block with a post comments link block inside the post template:
or use this sample code:

<!-- wp:query {"queryId":0,"query":{"perPage":3,"pages":0,"offset":0,"postType":"post","order":"desc","orderBy":"date","author":"","search":"","exclude":[],"sticky":"","inherit":true},"displayLayout":{"type":"flex","columns":3},"align":"wide","layout":{"type":"constrained"}} -->
<div class="wp-block-query alignwide"><!-- wp:post-template {"align":"wide"} -->
<!-- wp:post-title {"isLink":true} /-->

<!-- wp:post-comments-link {"style":{"elements":{"link":{"color":{"text":"var:preset|color|vivid-purple"}}}},"backgroundColor":"pale-pink"} /-->
<!-- /wp:post-template --></div>
<!-- /wp:query -->
Save and view the front. Confirm that the link color is still working and that the class name has-link-color is added.

Post blocks:

Sample query loop code:

<!-- wp:query {"queryId":0,"query":{"perPage":3,"pages":0,"offset":0,"postType":"post","order":"desc","orderBy":"date","author":"","search":"","exclude":[],"sticky":"","inherit":true},"displayLayout":{"type":"flex","columns":3},"align":"wide","layout":{"type":"constrained"}} -->
<div class="wp-block-query alignwide"><!-- wp:post-template {"align":"wide","style":{"elements":{"link":{"color":{"text":"var:preset|color|vivid-green-cyan"}}}}} -->
<!-- wp:paragraph -->
<p>The post title link should be purple:</p>
<!-- /wp:paragraph -->

<!-- wp:post-title {"isLink":true,"style":{"elements":{"link":{"color":{"text":"#b700ed"}}}}} /-->

<!-- wp:post-excerpt {"moreText":"Read more link text in post excerpt block should be red.","style":{"elements":{"link":{"color":{"text":"var:preset|color|vivid-red"}}}}} /-->

<!-- wp:paragraph -->
<p>This post date link should be yellow:</p>
<!-- /wp:paragraph -->

<!-- wp:post-date {"isLink":true,"style":{"elements":{"link":{"color":{"text":"var:preset|color|luminous-vivid-amber"}}}}} /-->

<!-- wp:paragraph -->
<p>This paragraph in the post template <a href="#">has a link</a>. The color should be green.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>The query pagination should have a light blue link color:</p>
<!-- /wp:paragraph -->
<!-- /wp:post-template -->

<!-- wp:query-pagination {"paginationArrow":"arrow","align":"wide","style":{"elements":{"link":{"color":{"text":"var:preset|color|pale-cyan-blue"}}}},"layout":{"type":"flex","justifyContent":"space-between"}} -->
<!-- wp:query-pagination-previous {"label":"Newer Posts"} /-->

<!-- wp:query-pagination-next {"label":"Older Posts"} /-->
<!-- /wp:query-pagination -->

<!-- wp:query-no-results -->
<!-- wp:paragraph {"placeholder":"Add text or blocks that will display when a query returns no results.","style":{"elements":{"link":{"color":{"text":"var:preset|color|vivid-cyan-blue"}}}}} -->
<p class="has-link-color">This is a paragraph inside the no results block. <a href="#">It should have a blue link color.</a></p>
<!-- /wp:paragraph -->
<!-- /wp:query-no-results --></div>
<!-- /wp:query -->

Various blocks:

  • To test the term description block, you need a term like a category that has a description with a link. Then add the term description block to an archive template, add the link color, and view that term.
  • In the block editor add one each of calendar, site title, and latests posts block.

Set a link color: it does not matter if it is a preset or custom color.
Save and view the front. Confirm that the link color is still working and that the class name has-link-color is now added to the wrapper.

@carolinan
Copy link
Contributor Author

@ockham Hi! I know you are very busy but can you confirm if it is not possible to add the has-link-color class with the style engine? I could not figure it out and make it work. Or, do you know who I can ping?

I believe adding the class directly like this through get_block_wrapper_attributes is what we were trying to avoid.

@carolinan
Copy link
Contributor Author

I could only find one block that uses gutenberg_style_engine_get_styles, the calendar, which is also missing the has-link-color class.

@ockham
Copy link
Contributor

ockham commented Jan 23, 2023

@ockham Hi! I know you are very busy but can you confirm if it is not possible to add the has-link-color class with the style engine? I could not figure it out and make it work. Or, do you know who I can ping?

Hey @carolinan! I don't really know myself how to do that, but maybe Style Engine experts @andrewserong, @ramonjd, or @scruffian could help? 😊

@andrewserong
Copy link
Contributor

if it is not possible to add the has-link-color class with the style engine

It's a little tricky this one, but I think the current shape of the style engine support doesn't quite cover link color (or elements) directly. The main functionality we currently have is either simple direct style values (like spacing properties, typography, etc), or for the more complex block supports (elements, layout) then the block support is responsible for passing values to the style engine and then the block support does its own parsing / output / working out where to put things.

So, for link color, from the perspective of the style engine, it doesn't really exist. Within elements.php the color is passed to the style engine, and is then returned, without the style engine having any idea that it's a link color instead of a regular color. For the style engine work, this is ideal, in that the style engine (to be well encapsulated) needs to not really know about anything other than simple style object values.

What does that mean for how we could approach this problem? At first glance, I think maybe the Elements block support (in elements.php) could handle injecting classnames dynamically, in much the same way as the Layout block support does for its classnames (the Layout support generates the classnames here, and then injects them here). Within the elements support, it looks like a good place to inject classnames could be around here where the elements class name is already being injected.

There is a caveat here, though: classnames for elements are currently being injected via WP_HTML_Tag_Processor, which is quite a neat way to handle it within the elements block support, however the tag processor might not be ready in time for 6.2 (see tracking issue here: #47349) so that's possibly not a good short-term solution depending on what happens there.

So, I think for now, I'd probably go with the manual approach of passing down the classnames as you've done here, then once the tag processor is ready to be used again, look at centralising the injecting of the classname within elements.php at the time that the elements classname is being injected.

What do you think?

@carolinan
Copy link
Contributor Author

carolinan commented Jan 24, 2023

Thank you for the detailed reply.
Yes that is what I arrived at too.
This PR is a temporary solution and only a small code change. I think we should include it and update it later.

@carolinan carolinan marked this pull request as ready for review January 24, 2023 05:48
@carolinan carolinan added the [Type] Enhancement A suggestion for improvement. label Jan 24, 2023
@github-actions
Copy link

Flaky tests detected in 6e5bf891c6580beb1a971b3cebd1c02b7a7c42f2.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/3994415609
📝 Reported issues:

@aristath aristath self-requested a review January 24, 2023 09:14
@aristath aristath merged commit 7880f08 into trunk Jan 25, 2023
@aristath aristath deleted the add/has-link-color branch January 25, 2023 09:45
@github-actions github-actions bot added this to the Gutenberg 15.1 milestone Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Several blocks are missing has-link-color class on the front
4 participants