Make WordPress Core

Opened 11 months ago

Closed 3 weeks ago

Last modified 3 weeks ago

#58022 closed defect (bug) (fixed)

Twenty Twenty: fix Table block's border colors

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

Description

Activate Twenty Twenty theme.
Choose Table block.
Change color of text.
Now see the difference both the side.
Editor side you don't have exact same color choosen as text color for border.
For user side it is working fine.
I have attached video for better understanding.
Video URL :- https://share.cleanshot.com/hpSKfF2yqyFphCnTDchy

Attachments (3)

58022.patch (1.0 KB) - added by nidhidhandhukiya 11 months ago.
After applying this solution the issue is resolved.
58022.2.patch (2.2 KB) - added by sabernhardt 11 months ago.
inherit border colors from table element if text color is normal; use currentColor when user chooses a text color
58022.alternative.patch (1022 bytes) - added by sabernhardt 8 months ago.
only fixes the default border color on the front

Download all attachments as: .zip

Change History (42)

@nidhidhandhukiya
11 months ago

After applying this solution the issue is resolved.

@sabernhardt
11 months ago

inherit border colors from table element if text color is normal; use currentColor when user chooses a text color

#1 @sabernhardt
11 months ago

  • Focuses css added
  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 6.3
  • Summary changed from Twenty Twenty table block having border color issue for editor side. to Twenty Twenty: table block's border color does not match user-selected text color
  • Version changed from 6.2 to 5.8

Twenty Twenty had that light border color on the front end for table blocks until the WordPress 6.1.1 minor release (higher specificity in GB45069, which may override other theme colors, too). The block did not have a custom text color option until 5.8.

58022.2.patch restores the theme's color on the front end, and the editor matches the color when a user selects a different text color. I used :where() to avoid increasing specificity.

#2 @pooja1210
11 months ago

Hi,

Test Report for - https://core.trac.wordpress.org/attachment/ticket/58022/58022.2.patch

Environment
WordPress - v6.2
Theme - Twenty Twenty

Actual:
Table border is not matching the user selected text color.

After Patch Expected Output:
Table block border color should match user selected text color ☑️

Screenshot:
Before adding the patch - https://prnt.sc/u8ULzAu2zO5E

After adding the patch - https://prnt.sc/2BzXb_7mlaJc

Thanks!

#3 @pouicpouic
11 months ago

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: https://core.trac.wordpress.org/ticket/58022

Environment

  • OS: Windows 10
  • Web Server: apache
  • PHP: 8.0.0
  • WordPress: 6.0-RC1-53341-src
  • Browser: Chrome 112.0.5615.138
  • Theme: Twenty Twenty
  • Active Plugins:
    • WordPress Beta Tester 3.3.7

Actual Results

  • ✅ Issue resolved with patch.

#4 @poena
8 months ago

  • Keywords 2nd-opinion added

I have tested 58022.2.patch on WordPress 6.3 alpha, and it works well in the editor and front.

Offering my second opinion: I would prefer not to change this because I would like the classic themes to be able to opt-in to use all the border controls instead. It would give users more options.
We would likely still need to make CSS changes if the border theme support is added: But if we change it to inherit the text color now, we would need to rewrite it twice.
https://github.com/WordPress/gutenberg/pull/47675

@sabernhardt
8 months ago

only fixes the default border color on the front

#5 @sabernhardt
8 months ago

Even if the custom colors need to wait for border color control to appear correctly in the editor, the default table borders should return to the theme's tan color on the front.

#6 @oglekler
8 months ago

  • Keywords needs-testing added; 2nd-opinion removed

New patch needs testing. It is simple enough and if it works as expected, it still can land into 6.3, especially due to its impact on only one theme.

#7 @shailu25
8 months ago

Test Report For https://core.trac.wordpress.org/attachment/ticket/58022/58022.alternative.patch

Environment:
============
WordPress - v6.2.2
Theme - Twenty Twenty

Backend: https://prnt.sc/AptH1OxWjKDB

Front-End :
===========
Before Patch: https://prnt.sc/j1kqH8Gfa2I1
After Patch : https://prnt.sc/sUJ4AhHQkMIW

Result After Patch:
================

  • Default Table border color Issue Resolved with patch in Frontend.
  • Default Table border color is displaying same as Backend.

Note: https://core.trac.wordpress.org/attachment/ticket/58022/58022.alternative.patch will work only fixes the default border color on the front (as per Mentioned), not user-selected text color

Last edited 7 months ago by shailu25 (previous) (diff)

This ticket was mentioned in Slack in #core by chaion07. View the logs.


8 months ago

#9 @ugyensupport
8 months ago

Twenty Twenty: table block's border color does not match user-selected text color

Description

Video URL :- https://share.cleanshot.com/hpSKfF2yqyFphCnTDchy

Environment

  • WordPress: 6.2.2
  • PHP: 8.0.0
  • Server: Apache/2.4.10 (Debian)
  • Database: mysqli (Server: 5.5.59-MariaDB-1~wheezy / Client: 5.5.62)
  • Browser: Chrome 114.0.0.0 (macOS)
  • Theme: Twenty Twenty 2.2
  • MU-Plugins: None activated
  • Plugins:
    • WordPress Beta Tester 3.5.0

Steps to Reproduce

  1. Activate Twenty Twenty theme.
  2. Choose Table block.
  3. Change color of text.
  4. 🐞 Bug occurs. Now see the difference both the side. Editor side you don't have exact same color chosen as the text color for the border. For the user side, it is working fine.

Expected Results

  1. ✅ After the patch it should work

Actual Results

  1. ❌ Patch not working on my end.

Frontend: https://file.io/MzfuIYEUtMcW
Before: https://file.io/5qdu15gfCg8F
After: https://file.io/9aYyQywD6EvA

Last edited 8 months ago by ugyensupport (previous) (diff)

#10 @poena
8 months ago

Hi @ugyensupport, which patch did you test in comment:9?

I did not personally find the time to start working on the full border support, I suggest committing 58022.2.patch. Unless that is the one that is broken in comment:9.

Last edited 8 months ago by sabernhardt (previous) (diff)

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


8 months ago

#13 @sabernhardt
8 months ago

  • Milestone changed from 6.3 to 6.4

It's quite late in the release cycle for this.

#14 @sabernhardt
7 months ago

#59134 was marked as a duplicate.

#15 @wasiur195
7 months ago

Twenty Twenty: table block's border color does not match user-selected text color

Description
Video URL :- https://drive.google.com/file/d/1FvIR0rfoIOpD7BW8pzWQrWQC1HM6nP6o/view?usp=sharing
Environment
WordPress: 6.4
PHP: 8.0.0
Server: Apache/2.4.10 (Debian)
Database: mysqli (Server: 5.5.59-MariaDB-1~wheezy / Client: 5.5.62)
Browser: Chrome 114.0.0.0 (macOS)
Theme: Twenty Twenty
MU-Plugins: None activated
Plugins:Default

Steps to Reproduce[]
Activate Twenty Twenty theme.
Choose Table block.
Change the color of the text.
Bug occurs. Now see the difference between both sides. Back End side, you don't have exact same color chosen as the text color for the border. For the user side, it is working fine.
Expected Results
✅ After the patch, it should work
Actual Results
❌ 58022.alternative.patch, not working on my end.
Screen Shot: https://prnt.sc/GpR9uv38SiKH

#16 @rajinsharwar
7 months ago

As the tests for the 58022.alternative.patch are failing for most of the peeps, I think we can commit the 58022.2.patch, unless there are any other strong reasons for it. Maybe we can finalize the testing for the 58022.2.patch?

#17 @wasiur195
7 months ago

  • Keywords needs-refresh changes-requested added; has-patch removed

@rajinsharwar and @sabernhardt
I have tried also 58022.2.patch on the latest WordPress version 6.4 but it shows an error on patch in my Code Editor. I think that it looks like merging issue and patch needs to be refreshed again.
Environment
WordPress: 6.4
Screenshot https://prnt.sc/Gb7CXDlemLpU

Last edited 7 months ago by sabernhardt (previous) (diff)

This ticket was mentioned in PR #5039 on WordPress/wordpress-develop by @sabernhardt.


7 months ago
#18

  • Keywords has-patch added; needs-refresh removed

Combines 58022.2.patch and 58022.alternative.patch, moving editor style change earlier in the stylesheet.

  • On the front end, the change restores the theme's default border color when (and only when) the block's text color is the default. This overrides Table block cell styles added in WordPress 6.1.1 (GB45069).
  • In the editor, these styles repurpose the user-selected color for the border, to match the front.

Editor with patch:
https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/17100257/77cf2e4a-fbcb-4902-b621-d1c08b0f4d6b

Front end with patch:
https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/17100257/a58d0e58-9c31-4fab-94d6-0596c028a219

Trac 58022

#19 @sabernhardt
7 months ago

  • Keywords changes-requested removed

I do not know why the other patches would have had a problem, but I think the PR combining both can be a better option.

#20 @wasiur195
6 months ago

  • Keywords needs-refresh changes-requested added; needs-testing removed

@sabernhardt I have combined the patches- 58022.2.patch and 58022.alternative.patch, but it provides an error. I am trying on WordPress 6.4. I think that it looks like a merging issue, and the patch needs to be refreshed again. Please see my attached video.

Video- https://drive.google.com/file/d/186alY0PIM4AymYDm9PxGVQQzf5DzPlvO/view?usp=sharing

#21 @sabernhardt
6 months ago

  • Keywords needs-testing has-testing-info added; needs-refresh changes-requested removed
  • Summary changed from Twenty Twenty: table block's border color does not match user-selected text color to Twenty Twenty: fix Table block's border colors

I could have said that more clearly: my pull request combines styles from the previous two patches.
Please test PR 5039 (5039.diff).

  1. Activate Twenty Twenty theme.
  2. Create a new post or page.
  3. Create at least two sets of three Table blocks. One set should have a header row, and another should not have header or footer rows.
  4. Leave one block as the default text color in each set, and change the text color for the other two blocks (select one color from the theme's palette and one custom color).
  5. Check the front end (preview) for discrepancies. Before applying the patch, the blocks with the default text color incorrectly would match the text color on the front, and the editor would give all of the Table blocks the theme's table border color (a light brown, #d7cfab, unless you made color changes in the Customizer).
  6. Apply the patch and refresh. If necessary, clear cache.
  7. Verify that the border colors in the editor match what is on the front end.

#22 @ugyensupport
6 months ago

great thanks @sabernhardt

#23 @jivygraphics
6 months ago

@sabernhardt Can confirm the patch working in local - screenshot attached:
https://jivygraphics.com/images/screencapture-localhost-8889-table-test-2023-08-24-14_45_02.png

Could not take a screenshot of the editor due to the iframe but it matched the front-end.

#24 @jivygraphics
6 months ago

  • Resolution set to worksforme
  • Status changed from new to closed

#25 @sabernhardt
6 months ago

  • Resolution worksforme deleted
  • Status changed from closed to reopened

Thanks for testing!

The 'worksforme' resolution is for bugs that cannot be reproduced.

Maybe your trouble with the iframe is merely a difficulty creating a screenshot of the entire page contents, but #59086 reports a bug for Twenty Twenty with dark background colors. If you want to turn off the iframe editor, you could try going to Preferences at the end of the 3-dot Options menu and enabling the Custom Fields panel (click the toggle and then the Show & Reload Page button that appears).

#26 @huzaifaalmesbah
6 months ago

Test Report

I Test Combines 58022.2.patch and 58022.alternative.patch ✅ working

Environment

OS: macOS m1
WordPress 6.4-alpha-56267-src
PHP 7.4.33
nginx/1.25.2
MySQL 5.7.43
Browser: Chrome 116.0.5845.140
Theme: Twenty Twenty v2.3
Active Plugins: No plugins activated.

Results

✅ Working

Editor

https://i.ibb.co/kKNyh2N/Screenshot-2023-09-15-at-11-44-47-PM.png

Frontend

https://i.ibb.co/fX1Vx5g/Screenshot-2023-09-15-at-11-44-33-PM.png

#27 @harshgajipara
5 months ago

  • Keywords has-screenshots added

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: https://core.trac.wordpress.org/attachment/ticket/58022/58022.2.patch

Environment:

OS: Windows
PHP: 8.1.9
WordPress: 6.3.1
Browser: Chrome
Theme: Twenty Twenty
Plugins: None activated

Actual Results:

Before Patch:
✅ In the editor border colors are not applied.
Backend: https://prnt.sc/Wa1jSreYl5ws
Frontend: https://prnt.sc/A2KXf8nijh65

After Patch:
✅ In the editor border colors are applied.
Backend: https://prnt.sc/_K3eTCTkhnsl
Frontend: https://prnt.sc/1lpWS_0Oja-V

Last edited 5 months ago by harshgajipara (previous) (diff)

#28 @ugyensupport
5 months ago

Description

Twenty Twenty: table block's border color does not match user-selected text color

Environment

  • WordPress: 6.3.1
  • PHP: 8.0.0
  • Server: Apache/2.4.10 (Debian)
  • Database: mysqli (Server: 5.5.59-MariaDB-1~wheezy / Client: 5.5.62)
  • Browser: Chrome 116.0.0.0 (macOS)
  • Theme: Twenty Twenty
  • MU-Plugins: None activated
  • Plugins:
    • WordPress Beta Tester 3.5.4

Steps to Reproduce

  1. Activate Twenty Twenty theme.
  2. Choose Table block.
  3. Change color of text.
  4. Now see the difference both the side.
  5. Editor side you don't have exact same color choosen as text color for border. For user side it is working fine.
  6. Here is the attached video for better understanding. Video URL :- https://share.cleanshot.com/hpSKfF2yqyFphCnTDchy

Expected Results

  1. ✅ After Patch tested: https://core.trac.wordpress.org/attachment/ticket/58022/58022.2.patch

Actual Results

  1. Backend: https://ibb.co/q75xcw7
  2. Frontend: https://ibb.co/Ryv6X3r
Last edited 5 months ago by ugyensupport (previous) (diff)

#29 @nicolefurlan
5 months ago

Testing Instructions

See #comment:21. We should use PR 5039 (5039.diff) for testing.

#30 @nicolefurlan
5 months ago

@huzaifaalmesbah, @harshgajipara, @ugyensupport would you be able to re-submit your test reports using the patch indicated in #comment:21?

This ticket was mentioned in Slack in #core by oglekler. View the logs.


5 months ago

#32 @nicolefurlan
5 months ago

@poena pinging you to see if you could rally some testers on this one as well, so we can hopefully get it resolved by RC1 next week :)

#33 @sabernhardt
5 months ago

  • Milestone changed from 6.4 to 6.5

@sumitbagthariya16 commented on PR #5039:


3 weeks ago
#34

### QA Update ✅

I have retested with 5039 Patch and it looks as expected.

Backend
https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/67687255/777cc784-9bff-4820-b5fb-a7be415d5a1a

Frontend
https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/67687255/a51ac4a1-b719-474c-8ca4-58c0d0e0a659

---

Testing Environment

  • WordPress: 6.5-alpha-56966-src
  • PHP: 8.0.0
  • Server: nginx/1.25.2
  • Browser: Chrome Version 121.0.6167.139 (Official Build) (x86_64) (macOS)
  • Theme: Twenty Twenty 2.5

#35 @sumitbagthariya16
3 weeks ago

  • Keywords needs-testing removed

#36 @poena
3 weeks ago

  • Keywords commit added

#37 @audrasjb
3 weeks ago

  • Owner set to audrasjb
  • Status changed from reopened to accepted

Thanks everyone, it looks good to me as well. Self assigning for commit.

#38 @audrasjb
3 weeks ago

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

In 57587:

Twenty Twenty: Fix Table block default and custom border colors.

On the front end, this changeset restores the theme's default border color when (and only when) the block's text color is the default.
In the editor, these styles repurpose the user-selected color for the border, to match the front.

Props nidhidhandhukiya, sabernhardt, pooja1210, pouicpouic, poena, shailu25, ugyensupport, wasiur195, rajinsharwar, wasiur195, jivygraphics, huzaifaalmesbah, harshgajipara, nicolefurlan, sumitbagthariya16.
Fixes #58022.

Note: See TracTickets for help on using tickets.