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

Fix for link color in containers #34689

Merged
merged 5 commits into from
Sep 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/block-supports/elements.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function gutenberg_render_elements_support( $block_content, $block ) {
}
$link_color_declaration = esc_html( safecss_filter_attr( "color: $link_color" ) );

$style = "<style>.$class_name a{" . $link_color_declaration . " !important;}</style>\n";
Copy link
Member Author

@oandregal oandregal Sep 9, 2021

Choose a reason for hiding this comment

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

This is going to need changes in the tests. Will update them once we're certain this is the right direction.

Copy link
Member Author

@oandregal oandregal Sep 9, 2021

Choose a reason for hiding this comment

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

This is going to have 011 specificity.

I've looked at all current cases of core blocks and this change will still work well with global styles. The reason is that the current global styles generated for links have 011 or 002, depending on whether they are .wp-block-* a or element a. The element styles are loaded after the global styles, so the element styles win.

I was thinking that we could add the :not(.has-link-color) selector to the global styles we generate. Should any block have higher than 011 specificity this selector will prevent it from applying. However! This is also not complete: it won't work for some blocks that declare __experimentalSelector and target an inner element. Use case:

  • some block has a wrapper such as .wp-block-wrapper
  • for whatever reason, the block declares that the styles should match some inner element instead __experimentalSelector: .wp-block-wrapper-inner .wp-block-wrapper-extra

Adding the negation in this situation would render .wp-block-wrapper-inner .wp-block-wrapper-extra:not(.has-link-color) { /* link color */ } which has higher specificity that the link color element and still targets the link in the block because the .has-link-color class lives in the wrapper.

$style = "<style>.$class_name a{" . $link_color_declaration . ";}</style>\n";

// Like the layout hook this assumes the hook only applies to blocks with a single wrapper.
// Retrieve the opening tag of the first HTML element.
Expand All @@ -59,7 +59,7 @@ function gutenberg_render_elements_support( $block_content, $block ) {
$first_element_offset = $html_element_matches[0][1];
$content = substr_replace( $block_content, ' class="' . $class_name . '"', $first_element_offset + strlen( $first_element ) - 1, 0 );
}
return $content . $style;
return $style . $content;

}

Expand Down
4 changes: 1 addition & 3 deletions packages/block-editor/src/hooks/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,7 @@ function compileElementsStyles( selector, elements = {} ) {
...map(
elementStyles,
( value, property ) =>
`\t${ kebabCase( property ) }: ${ value }${
element === 'link' ? '!important' : ''
};`
`\t${ kebabCase( property ) }: ${ value };`
),
'}',
].join( '\n' );
Expand Down
6 changes: 5 additions & 1 deletion packages/block-library/src/paragraph/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ p.has-background {
padding: $block-bg-padding--v $block-bg-padding--h;
}

p.has-text-color a {
// Use :where to contain the specificity of this rule
// so it's easily overrideable by any theme that targets
// links using the a element.
// For example, this is what global styles does.
:where(p.has-text-color:not(.has-link-color)) a {
color: inherit;
}
6 changes: 3 additions & 3 deletions phpunit/class-elements-test.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public function test_simple_paragraph_link_color() {
);
$this->assertSame(
$result,
'<p class="wp-elements-1">Hello <a href="http://www.wordpress.org/">WordPress</a>!</p><style>.wp-elements-1 a{color: var(--wp--preset--color--subtle-background) !important;}</style>' . "\n"
'<style>.wp-elements-1 a{color: var(--wp--preset--color--subtle-background);}</style>' . "\n" . '<p class="wp-elements-1">Hello <a href="http://www.wordpress.org/">WordPress</a>!</p>'
);
}

Expand Down Expand Up @@ -71,7 +71,7 @@ public function test_class_paragraph_link_color() {
);
$this->assertSame(
$result,
'<p class="wp-elements-1 has-dark-gray-background-color has-background">Hello <a href="http://www.wordpress.org/">WordPress</a>!</p><style>.wp-elements-1 a{color: red !important;}</style>' . "\n"
'<style>.wp-elements-1 a{color: red;}</style>' . "\n" . '<p class="wp-elements-1 has-dark-gray-background-color has-background">Hello <a href="http://www.wordpress.org/">WordPress</a>!</p>'
);
}

Expand Down Expand Up @@ -100,7 +100,7 @@ public function test_anchor_paragraph_link_color() {
);
$this->assertSame(
$result,
'<p id="anchor" class="wp-elements-1">Hello <a href="http://www.wordpress.org/">WordPress</a>!</p><style>.wp-elements-1 a{color: #fff000 !important;}</style>' . "\n"
'<style>.wp-elements-1 a{color: #fff000;}</style>' . "\n" . '<p id="anchor" class="wp-elements-1">Hello <a href="http://www.wordpress.org/">WordPress</a>!</p>'
);
}
}