Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#52555 closed defect (bug) (fixed)

Twenty Nineteen: Link-based buttons do not have a text color set in the editor

Reported by: mikejolley's profile mikejolley Owned by: ryelle's profile ryelle
Milestone: 5.8 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch has-testing-info commit
Focuses: css Cc:

Description

In the editor (not on the frontend) if you have a link-based button, the text color is not set. This means it inherits the default link color (blue) on top of a blue background.

This is not a problem on the frontend because .wp-block-button__link has the text color set the white.

To fix this, we can use similar CSS in the editor, setting the text color to white.

Screenshots and patch to follow...

Attachments (2)

102087854-68028700-3e1a-11eb-9b8f-fc7f623668b8.png (434.6 KB) - added by mikejolley 3 years ago.
Screenshot of issue
preview.gif (29.3 KB) - added by poena 3 years ago.
Missing CSS in HTML Block preview

Download all attachments as: .zip

Change History (22)

This ticket was mentioned in PR #1012 on WordPress/wordpress-develop by mikejolley.


3 years ago
#1

  • Keywords has-patch added

This adds a text color for the .wp-block-button__link element in the editor. This is already done in the frontend css, just not within the editor css.

Trac ticket: https://core.trac.wordpress.org/ticket/52555

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


3 years ago

#3 @hellofromTonya
3 years ago

  • Version trunk deleted

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


3 years ago

#5 @poena
3 years ago

Hi @mikejolley

Twenty Nineteen gets the text color for .wp-block-button__link in the editor from the
block-library stylesheet.

Since this is an issue with the WooCommerce button, would this not be fixed here?
https://github.com/woocommerce/woocommerce/blob/trunk/assets/css/twenty-nineteen.scss

#6 @mikejolley
3 years ago

@poena We can work around this in our CSS by forcing "white" text, however, we though it would be best to do this upstream to avoid theme specific workarounds. There is a definite discrepancy between the default styles used on the frontend vs the editor.

This problem is not specific to WooCommerce though. If you add the button markup to a HTML block (<div class="wp-block-button"><a href="#" class="wp-block-button__link">Test</a></div>) and preview in the editor compared to previewing the frontend, you'll see also the blue text in editor, but white text on frontend. This doesn't affect the "Button" block because that injects some other inline classnames and elements.

#7 @poena
3 years ago

  • Milestone changed from Awaiting Review to 5.8

I do not see the styles applied in the HTML block Preview option, but I recognize that that is not a theme issue.

I don't believe that your PR would have any unintended side effects, but if you want this to be tested further please include instructions for reproducing the issue.

#8 @mikejolley
3 years ago

  • Focuses accessibility removed

I do not see the styles applied in the HTML block Preview option, but I recognize that that is not a theme issue.

Can you elaborate on this point please?

In the HTML block preview, if the theme styles were correct you'd see blue buttons with white text, just like you see on the frontend after publishing.

This is the issue we're seeing with the block in WooCommerce. If you insert or use the exact same HTML that the button block renders, the text is blue rather than white, because there is a discrepancy between editor styles and frontend styles.

Edit, I removed the accessibility tag by accident sorry!

Last edited 3 years ago by mikejolley (previous) (diff)

#9 @poena
3 years ago

No theme or editor styles are added to the HTML block preview, this is what I am trying to explain:
https://github.com/WordPress/gutenberg/issues/9129

That is why other steps to reproduce this issue are needed.

Last edited 3 years ago by poena (previous) (diff)

#10 follow-up: @mikejolley
3 years ago

@poena If that were the case we wouldn't see the blue button styling at all in the preview right? I'm assuming Twenty Nineteen does already handle this.

It's difficult to include any other testing instructions—the only way to reproduce this issue is to have a block which implements buttons using the same markup generated by the button block. So the original example in WooCommerce which renders multiple buttons as part of product views, or using button markup directly as HTML in the HTML block.

As I said before, the actual button block is not affected by this issue because it uses different markup in the editor to that rendered in the frontend.

For what it's worth, WooCommerce uses the same markup for buttons as the button block so that buttons inherit styling from the theme. Personally I see no issue with this, and the other default themes have no issue with this either.

Other 3rd party Blocks which need to render buttons would likely need to do something similar, so if this is not fixed upstream in Twenty Nineteen, they'll encounter the same issue as we did and need to style the editor in Twenty Nineteen specifically to work around it.

I hope this makes the justification for this one liner a little clearer. If you don't think this needs fixing in Twenty Nineteen please close the issue. We have a workaround in WooCommerce now anyway.

#11 in reply to: ↑ 10 @poena
3 years ago

  • Keywords needs-testing added

Replying to mikejolley:

@poena If that were the case we wouldn't see the blue button styling at all in the preview right? I'm assuming Twenty Nineteen does already handle this.

Correct, I am not seeing the blue button styling at all in the preview.

Please don't insinuate that I am delaying your patch. I specifically wrote that I don't think the patch causes any side effects. Patches to core are not instantly merged, unless critical (I am also not a committer).
What I am, is unable to properly test the patch. This does not mean that someone else can't test it and mark the issue for commit, with proper instructions.

@poena
3 years ago

Missing CSS in HTML Block preview

#12 @mikejolley
3 years ago

Hmm thats puzzling because I am certain I tried this before and had the behaviour I described :( I must have been looking at another button.

Please don't insinuate that I am delaying your patch.
What I am, is unable to properly test the patch.

Sorry if I came across that way. I am frustrated with how difficult it is to replicate this with only WP and no 3rd party plugins.

It's probably not ideal to get people to install an old version of Woo and Woo Blocks to test this so I've written a small snippet to add a SSR block which just renders a button directly in the editor.

If you add this to theme functions.php, and then insert the bug/button-styling block into a page you should immediately see a blue button with blue text in the editor, and a blue button with white text in the frontend. Do not click the button because it appears that ":visited" links do have correct white styling.

Snippet: https://gist.github.com/mikejolley/111f4b80a77a1897964388b08d419b31
Editor screenshot: https://user-images.githubusercontent.com/90977/112491400-2ddd4580-8d78-11eb-85d4-0d46c79bd1a8.png
Frontend screenshot: https://user-images.githubusercontent.com/90977/112491395-2d44af00-8d78-11eb-929b-c03f5236fdbb.png

After applying the patch, you will see white text on blue background in the editor as well. The CSS I am adding to the editor stylesheet is copied from the frontend stylesheet.

#13 @Boniu91
3 years ago

  • Keywords has-testing-info added

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


3 years ago

#15 @Boniu91
3 years ago

@mikejolley When applying the PR, the button text is, indeed styled correctly. However, I'm seeing the following errors, only with the PR applied:

Warning: file_get_contents(/var/www/src/wp-includes/css/dist/editor/editor-styles.css): failed to open stream: No such file or directory in /var/www/src/wp-admin/edit-form-blocks.php on line 191

Warning: file_get_contents(/var/www/src/wp-includes/css/dist/editor/editor-styles.css): failed to open stream: No such file or directory in /var/www/src/wp-admin/edit-form-blocks.php on line 227

Warning: Cannot modify header information - headers already sent by (output started at /var/www/src/wp-admin/edit-form-blocks.php:191) in /var/www/src/wp-admin/admin-header.php on line 9

Tested on:
NGINX
PHP7.4
WordPress 5.7-beta3-50368-src (version from PR)

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


3 years ago

#17 @ryelle
3 years ago

  • Keywords commit added

@mikejolley thanks for adding the gist for testing, that's really helpful. Confirming that the patch makes the editor & frontend match.

#18 @ryelle
3 years ago

  • Keywords needs-testing removed

#19 @ryelle
3 years ago

  • Owner set to ryelle
  • Resolution set to fixed
  • Status changed from new to closed

In 51098:

Twenty Nineteen: Set a default color for button links in the editor.

Props mikejolley, poena, boniu91.
Fixes #52555.

This ticket was mentioned in Slack in #core-test by lucatume. View the logs.


3 years ago

Note: See TracTickets for help on using tickets.