Make WordPress Core

Opened 2 years ago

Closed 7 months ago

Last modified 7 months ago

#57544 closed defect (bug) (fixed)

Twenty Twenty: Incorrect Colour in editor for Separator Block

Reported by: bhaveshdesai13's profile bhaveshdesai13 Owned by: karmatosed's profile karmatosed
Milestone: 6.7 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch needs-testing commit
Focuses: css Cc:

Description

The colour selection in the Separator Block settings shows incorrectly. The colour shown in the block setting is different from the one the separator has in the editor and the front end.
Please refer to the video link for a better understanding: https://share.cleanshot.com/8PpMy6fxLxRJ2KbrY8Gm
Bug Screenshot: https://share.cleanshot.com/pgGnkPC03PYbzZNtS9ml

Also, the sepeator behaves weirdly on clicking it: https://share.cleanshot.com/BpSRGJX7f8MYVw80rcGD

Attachments (4)

57544.patch (2.2 KB) - added by nidhidhandhukiya 2 years ago.
57544.1.patch (3.8 KB) - added by sabernhardt 20 months ago.
keeping custom colors together with extra-high specificity for Separator block (:root .wp-block-separator.has-accent-color)
57544.2.patch (1.6 KB) - added by sabernhardt 20 months ago.
repeating custom color classes in the Separator block section for lower specificity (.wp-block-separator.has-accent-color)
57544.3.patch (5.5 KB) - added by sabernhardt 7 months ago.
rearranges the rulesets to group custom background color classes before the text color classes

Download all attachments as: .zip

Change History (19)

#1 @aniketpatel
2 years ago

Thanks, @bhaveshdesai13 for the ticket!
Good catch.
It seems it is not working properly in WP 6.1.1 with the Gutenberg editor.

#2 @miguelaxcar
2 years ago

Thanks @bhaveshdesai13 , I managed to reproduce the issue on WP 6.1.1.

I tried fixing locally but it looks like lift heavier than expected, as the CSS has a different format on twenty-twenty theme compared to twenty-twenty-one theme, for instance.

On twenty-twenty-one theme the CSS uses variables for colors, what would be good adding to twenty-twenty theme IMHO, and also, the selectors are different for applying the proper properties to .has-COLORNAME-color and .has-COLORNAME-background-color.

I'm not the most experienced frontend WP engineers, so let's wait for more seasoned fellows chiming in.

#3 @bhaveshdesai13
2 years ago

  • Keywords needs-testing has-patch added; has-screenshots needs-patch removed

Thank you for the patch @nidhidhandhukiya.

The colour shown in the block settings for the Separator Block is now the same as the one the separator has in the editor and the front end.

Separator Block in Editor and front end after the patch: https://share.cleanshot.com/CqNdz9qTnVllCQ0N5YzH

#4 @poena
2 years ago

  • Keywords needs-refresh added

Hi!

The patch changes the color for root; kindly update it to only apply the palette colors to the separator block.

#5 @pavanpatil1
20 months ago

Test Report

Test report for - https://core.trac.wordpress.org/attachment/ticket/57544/57544.patch

Environment
WordPress - V6.2.2
Theme - Twenty Twenty
Os - Windows
Browser - Chrome

Screenshot

Before: https://prnt.sc/seimRXQAVCMm

After: https://prnt.sc/218c3hwk0C4W

The patch is working as expected, However, as suggested in above comment it is changing the color for .root. Need to update it accordingly.

@sabernhardt
20 months ago

keeping custom colors together with extra-high specificity for Separator block (:root .wp-block-separator.has-accent-color)

@sabernhardt
20 months ago

repeating custom color classes in the Separator block section for lower specificity (.wp-block-separator.has-accent-color)

#6 @sabernhardt
20 months ago

  • Focuses css added
  • Keywords needs-refresh removed

I tried grouping the Separator block selectors in the Custom Colors section, but that required a higher specificity than putting them all further down in the Separator block section. 57544.2.patch is probably the better option.

Also, the separator behaves weirdly on clicking it

The focus style is another issue, which could be easier to fix in the editor (GB41543).

#7 @bhaveshdesai13
20 months ago

  • Keywords needs-testing removed

Thank You for the Patch @sabernhardt

Test Report
Test report for - https://core.trac.wordpress.org/attachment/ticket/57544/57544.2.patch

Environment
WordPress - v6.2.2
Theme - Twenty Twenty
OS - macOS Ventura 13.4.1
Browser - Chrome v114.0.5735.133

Screen Record Before and After the Patch
Before: https://share.cleanshot.com/1k0zXz00k8cXV3zjp9D3
After: https://share.cleanshot.com/lhYF9yTlh2QmD9cJXjyD

The Separator Blocks colors are now the same in the block settings, editor, and frontend.

#8 @karmatosed
7 months ago

Assigning so I can go through this test and look towards if possibly this could be committed, thank you everyone.

#9 @karmatosed
7 months ago

  • Milestone changed from Awaiting Review to 6.7
  • Owner set to karmatosed
  • Status changed from new to assigned

#10 @karmatosed
7 months ago

  • Keywords close added
  • Milestone changed from 6.7 to Future Release

I just went through and tested the following and got it without the patch working. I would appreciate a check to see if this is the case though for everyone. It might be due to time and other factors I might be missing this has been fixed or I am not replicating correctly though.

  1. Activated seperator.
  2. Changed the color.

Expected result based on bug: to not have the front and back same color change for background.
Result: the same color.

#11 @karmatosed
7 months ago

  • Owner karmatosed deleted

@sabernhardt
7 months ago

rearranges the rulesets to group custom background color classes before the text color classes

#12 @sabernhardt
7 months ago

  • Keywords needs-testing added; close removed

The theme preset colors still can display incorrectly in the editor, but the stylesheets should not need to add new styles. The order of rulesets can cause problems. The theme's automatic contrast text color with a background-related class should come before the rules for text colors (at the same specificity level). Then the Separator would inherit its color from the text color class.

57544.3.patch edits the order for the front end too, though the inline twentytwenty_get_customizer_css colors tend to override style.css.

My sample blocks:

  • Separator blocks with default color, Twenty Twenty palette presets, black and white presets, and one custom color (lighter colors are in a Group with a darker background for better visibility)
  • Paragraph block with Accent text color and Primary background color

#13 @karmatosed
7 months ago

  • Milestone changed from Future Release to 6.7
  • Owner set to karmatosed

Thank you for both the patch and also for the sample blocks @sabernhardt. I am going to test and look to get this brought in.

#14 @karmatosed
7 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 58784:

Twenty Twenty: Fixes incorrect colour in editor for seperator block.

This resolves the seperator block color settings ordering. They were displaying incorrectly due to ruleset problems. This edits the order for the front end also.

Props bhaveshdesai13, aniketpatel, miguelaxcar, nidhidhandhukiya, poena, pavanpatil1, sabernhardr.
Fixes #57544.

#15 @karmatosed
7 months ago

  • Keywords commit added
Note: See TracTickets for help on using tickets.