-
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
Link styles: underline enforcement overrides browser preferences #63647
Comments
Thanks for raising this one @markhowellsmead 👍 I'm unsure of the best path forward here. The point you make regarding the potential override of user/browser preferences for accessibility reasons is a good one. On the other hand, removing this style definition from the core @richtabor do you have any wisdom you can share that might help tip the scales here? |
Hypothetically, should this remove all the styles that ship in the default theme.json? // I don't work, don't copy/paste me!
add_filter("wp_theme_json_data_default", function ($theme_json) {
$data = $theme_json->get_data();
// Remove default color palette.
$data["settings"]["color"]["palette"]["default"] = [];
// Remove default duotone.
$data["settings"]["color"]["duotone"]["default"] = [];
// Remove default gradients.
$data["settings"]["color"]["gradients"]["default"] = [];
// Remove default styles.
$data["styles"] = [];
// Update the theme data.
$theme_json->update_with($data);
return $theme_json;
}); In my testing, it does remove all the default colors/gradients/etc, but the styles defined here are still included in |
@cbirdsong the theme json I'm not sure that we can alter the logic for styles in a backwards-compatible manner. To your main point though about the possibility of using a filter if you wish to opt out of these defaults for links. That should be achievable with something like this: add_filter(
'wp_theme_json_data_default',
function ( $theme_json ) {
$data = $theme_json->get_data();
// Remove default link decoration styles.
$data['styles'] = array(
'elements' => array(
'link' => array(
'typography' => array(
'textDecoration' => 'initial',
),
':hover' => array(
'typography' => array(
'textDecoration' => 'initial',
),
),
),
),
);
// Update the theme data.
$theme_json->update_with( $data );
return $theme_json;
},
); @markhowellsmead does the above snippet help address your immediate need? |
@aaronrobertshaw The issue is more of a matter of principle right now, although I’ve already implemented a filtering solution to theme.json on a number of current projects. |
Doing that outputs the following: a:where(:not(.wp-element-button)) {
text-decoration: initial;
}
a:where(:not(.wp-element-button)):hover {
text-decoration: initial;
} This still causes issues overriding default link styles. Setting the contents to "" seems to work okay: $data["styles"] = [
"elements" => [
"link" => [
"typography" => [
"textDecoration" => "",
],
],
],
]; (I'm also not sure the However, looking at the bigger picture, in order to get rid of this: .wp-element-button, .wp-block-button__link {
background-color: #32373c;
border-width: 0;
color: #fff;
font-family: inherit;
font-size: inherit;
line-height: inherit;
padding: calc(0.667em + 2px) calc(1.333em + 2px);
text-decoration: none;
} I would have look at the default theme.json and then to do this: $data["styles"] = [
"elements" => [
"button" => [
"color" => [
"color" => "",
"background" => "",
],
"padding" => [
"top" => "",
"right" => "",
"bottom" => "",
"left" => "",
],
// etc, etc
],
],
]; Then I'd have to do that again on every site every time this file changes. Is there really no way to do this more systematically? |
@aaronrobertshaw How practical would it be to add another function that replaces instead of merges, like I'm sure there are edge cases I haven't thought of, but it's very unintuitive that passing an empty doesn't work.
Would be curious to see anything if you are able to share! |
@markhowellsmead I definitely understand the argument 👍 Given sites might be relying on those default styles now, I'm leaning towards a pragmatic approach. I appreciate it isn't ideal but the status quo doesn't impact existing sites that might depend on these defaults and the workaround to filter them out is available to address the concern expressed in this issue. Should there be widespread consensus that the risk of removing these defaults is worth it, perhaps we can find a means of deprecating them or making them opt-in. I'm not sure how that can be achieved in a backward compatible manner though. Maybe it would require a new theme.json version so the migration of older versions could inject the deprecated defaults.
@cbirdsong, my apologies, I pasted in the wrong iteration of my test code. Thank you for correcting it to the empty string option.
Taking a step back, rather than overriding values in the default theme.json data, it should be possible to completely discard the defaults. The following might provide an option for you to explore: add_filter(
'wp_theme_json_data_default',
function ( $theme_json ) {
return new WP_Theme_JSON_Data(
array( 'version' => WP_Theme_JSON::LATEST_SCHEMA ),
'default'
);
}
); You can also then set the priority for the filter to ensure it's run last etc.
I'm honestly not sure. While it does provide some desired control it also means there's differing ways to modify that data that could make debugging issues in a very complex area much harder. I'm not trying to suggest it is impossible but that all the pros and cons would need to be weighed to determine whether it is feasible and beneficial in the long run.
Agreed. Once the presets were handled differently it certainly made the behaviour less predictable. |
Description
The definition at
gutenberg/lib/theme.json
Lines 360 to 364 in d1211ff
Step-by-step reproduction instructions
See above
Screenshots, screen recording, code snippet
No response
Environment info
Please confirm that you have searched existing issues in the repo.
Please confirm that you have tested with all plugins deactivated except Gutenberg.
The text was updated successfully, but these errors were encountered: