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 block border color issues in bundled themes (Pullquote) #2944

Open
wants to merge 12 commits into
base: trunk
Choose a base branch
from

Conversation

adamwoodnz
Copy link

@adamwoodnz adamwoodnz commented Jul 5, 2022

This PR fixes issues with block border colors in the bundled Twenty Twenty One, Twenty Twenty and Twenty Nineteen themes, in particular the Pullquote block.

Currently when a border color is set from the theme color palette for a Pullquote block, it is shown in the editor but not on the frontend. This includes when these colors are changed through the customizer. Custom colors selected through the border color block setting are applied correctly.

Style rules have been added targeting classes already applied to the blocks, eg. .has-primary-border-color. As such the effects will not be limited to the Pullquote block, but I'm yet to find another block affected.

Closes https://core.trac.wordpress.org/ticket/55974

NOTE: Testing notes on the ticket also mention that there are issues with Twenty Sixteen and Twenty Fifteen, but this PR does not address those.

Screenshots

Twenty Twenty One

Editor default Frontend default
Screen Shot 2023-02-24 at 1 56 43 PM Screen Shot 2023-02-24 at 1 59 09 PM

Twenty Twenty

Editor default Frontend default Editor customized Frontend customized
Screen Shot 2023-02-24 at 2 00 46 PM Screen Shot 2023-02-24 at 2 01 07 PM Screen Shot 2023-02-24 at 2 02 39 PM Screen Shot 2023-02-24 at 2 02 52 PM

Twenty Nineteen

Editor default Frontend default Editor customized Frontend customized
Screen Shot 2023-02-24 at 1 48 49 PM Screen Shot 2023-02-24 at 1 49 11 PM Screen Shot 2023-02-24 at 1 52 15 PM Screen Shot 2023-02-24 at 1 52 29 PM

Testing

Tested in Chrome, Firefox, Safari and Edge on MacOS.

Steps
For each of the themes Twenty Twenty One, Twenty Twenty and Twenty Nineteen:

Default colors

  1. Create a post with a Pullquote block
  2. Set the border color of the Pullquote using the block options
  3. Ensure the border color is displayed in the editor
  4. Publish the post and ensure the border color is displayed in the frontend

Customized theme colors (not supported in Twenty Twenty One)

  1. Go to the theme customizer and customize any of the colors allowed by the theme
  2. Return to your post and check that both the editor and the published post use the customized theme color

Custom block colors

  1. Set a custom color for the selected block using the block border settings, eg. enter #FF0000
  2. Return to your post and check that both the editor and frontend use the same custom color

@github-actions
Copy link

github-actions bot commented Jul 5, 2022

Hi @adamwoodnz! 👋

Thank you for your contribution to WordPress! 💖

It looks like this is your first pull request to wordpress-develop. Here are a few things to be aware of that may help you out!

No one monitors this repository for new pull requests. Pull requests must be attached to a Trac ticket to be considered for inclusion in WordPress Core. To attach a pull request to a Trac ticket, please include the ticket's full URL in your pull request description.

Pull requests are never merged on GitHub. The WordPress codebase continues to be managed through the SVN repository that this GitHub repository mirrors. Please feel free to open pull requests to work on any contribution you are making.

More information about how GitHub pull requests can be used to contribute to WordPress can be found in this blog post.

Please include automated tests. Including tests in your pull request is one way to help your patch be considered faster. To learn about WordPress' test suites, visit the Automated Testing page in the handbook.

If you have not had a chance, please review the Contribute with Code page in the WordPress Core Handbook.

The Developer Hub also documents the various coding standards that are followed:

Thank you,
The WordPress Project

@adamwoodnz adamwoodnz changed the title Theme-specific border color options for blocks WIP: Theme-specific border color options for blocks Jul 5, 2022
@adamwoodnz adamwoodnz marked this pull request as draft July 5, 2022 05:17
@adamwoodnz adamwoodnz changed the title WIP: Theme-specific border color options for blocks Theme-specific border color options for blocks Jul 5, 2022
@adamwoodnz adamwoodnz marked this pull request as ready for review July 5, 2022 22:40
@adamwoodnz adamwoodnz force-pushed the fix/55974-theme-border-colors branch from 27ea788 to 281ba6e Compare August 16, 2022 23:51
@adamwoodnz adamwoodnz force-pushed the fix/55974-theme-border-colors branch 3 times, most recently from 5cc4a96 to 3561188 Compare February 23, 2023 01:33
@adamwoodnz adamwoodnz force-pushed the fix/55974-theme-border-colors branch 2 times, most recently from a24d9ca to 3f42dec Compare February 23, 2023 04:12
@adamwoodnz adamwoodnz marked this pull request as draft February 24, 2023 01:09
@adamwoodnz adamwoodnz marked this pull request as ready for review February 24, 2023 01:09
@adamwoodnz adamwoodnz changed the title Theme-specific border color options for blocks Fix border color issues in bundled themes (Pullquote) Feb 24, 2023
@adamwoodnz adamwoodnz changed the title Fix border color issues in bundled themes (Pullquote) Fix block border color issues in bundled themes (Pullquote) Feb 24, 2023
@adamwoodnz adamwoodnz force-pushed the fix/55974-theme-border-colors branch from 3f42dec to 64bfa9b Compare July 24, 2023 03:54
@carolinan
Copy link

carolinan commented Jul 24, 2023

Thank you for continuing the work on this feature.
I want to add that from WordPress 6.3, themes can opt-in to the block border support with: add_theme_support( 'border' );
I think the default themes (non block themes) should use this theme support.

The CSS changes are still needed. I have opened a related issue in the Gutenberg GitHub repo:
WordPress/gutenberg#52864

@adamwoodnz
Copy link
Author

You're welcome!

I want to add that from WordPress 6.3, themes can opt-in to the block border support with: add_theme_support( 'border' );
I think the default themes (non block themes) should use this theme support.

To be clear; do you mean there is more work to do in this PR?

@carolinan
Copy link

Yes I think the theme support should be added to the functions.php files setup functions. Otherwise the user can not take full advantage of the CSS border changes?

@adamwoodnz
Copy link
Author

adamwoodnz commented Jul 24, 2023

Otherwise the user can not take full advantage of the CSS border changes?

So will that mean that any block that supports border added in these themes, will now get border controls displayed? How far back do you think we should add this support.. beyond twentynineteen?

@adamwoodnz
Copy link
Author

So will that mean that any block that supports border added in these themes, will now get border controls displayed?

Ah yes I can see this working after adding it.

@adamwoodnz adamwoodnz force-pushed the fix/55974-theme-border-colors branch 2 times, most recently from 5f71b29 to 4722757 Compare July 24, 2023 05:42
@carolinan
Copy link

Lets start with these 3 themes because the PR has the potential to grow too big.

@adamwoodnz
Copy link
Author

adamwoodnz commented Jul 25, 2023

Discovered that twentytwenty actually has border styles off for the pullquote block. It seems that the fix should be to disable the controls in that scenario.

It also has no padding so if you change background color it doesn't look great:

Screenshot 2023-07-25 at 8 27 11 PM

We could allow border and improve the styling by changing the styles to something like:

.editor-styles-wrapper .wp-block-pullquote {
	color: inherit;
	position: relative;
	text-align: center;
}

.editor-styles-wrapper .wp-block-pullquote:not(.has-border-color):not([style*=border-width]) {
	border: none;
	padding: 0;
}

Thoughts @carolinan ?

@adamwoodnz
Copy link
Author

Discovered that twentytwenty actually has border styles off for the pullquote block. It seems that the fix should be to disable the controls in that scenario.

I was wrong this was only in the editor. Added a fix to add editor support. Haven't addressed padding.

@adamwoodnz
Copy link
Author

I've now tested the 3 themes with pullquote and group blocks, including changing colors with customizer and custom block colors, and I believe all are working as expected. Are there any other blocks in particular that you think warrant testing?

@adamwoodnz adamwoodnz force-pushed the fix/55974-theme-border-colors branch 2 times, most recently from afa1d81 to 2cb9c56 Compare July 25, 2023 22:05
@adamwoodnz
Copy link
Author

These E2E tests seem very flaky; failures due to TimeoutError: Navigation timeout of 30000 ms exceeded seem very common on https://github.com/WordPress/wordpress-develop/actions/workflows/end-to-end-tests.yml

@adamwoodnz
Copy link
Author

I've just rebased again @carolinan, are you able to give this another review please? Pretty keen to get this wrapped up 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Review (PR only)
2 participants