#52555 closed defect (bug) (fixed)
Twenty Nineteen: Link-based buttons do not have a text color set in the editor
Reported by: | mikejolley | Owned by: | 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)
Change History (22)
This ticket was mentioned in PR #1012 on WordPress/wordpress-develop by mikejolley.
4 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.
4 years ago
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
4 years ago
#5
@
4 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
@
4 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
@
4 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
@
4 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!
#9
@
4 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.
#10
follow-up:
↓ 11
@
4 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
@
4 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.
#12
@
4 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.
This ticket was mentioned in Slack in #core-test by boniu91. View the logs.
4 years ago
#15
@
4 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.
4 years ago
#17
@
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.
#19
@
3 years ago
- Owner set to ryelle
- Resolution set to fixed
- Status changed from new to closed
In 51098:
Screenshot of issue