Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 22 months ago

#57354 closed enhancement (fixed)

Add outline support for blocks via theme.json

Reported by: onemaggie's profile onemaggie Owned by: hellofromtonya's profile hellofromTonya
Milestone: 6.2 Priority: normal
Severity: normal Version:
Component: Themes Keywords: has-patch has-unit-tests has-testing-info commit gutenberg-merge
Focuses: Cc:

Description (last modified by hellofromTonya)

Adds the ability to define outline CSS properties for elements and blocks within theme.json to render outline-color, outline-offset, outline-style, and outline-width styles.

Reference:

Note: this enhancement was missed during the 6.1 release.

Attachments (2)

trac-57354.gif (3.2 MB) - added by hellofromTonya 2 years ago.
Test Report results: worked as expected in Chrome
tt3-button-border-test.diff (782 bytes) - added by ironprogrammer 2 years ago.
TT3 modification to add outline and hover styles to button element

Change History (30)

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


2 years ago
#1

  • Keywords has-patch added

This PR backports the changes in #43526 that add support for the outline property on theme.json

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

@audrasjb commented on PR #3788:


2 years ago
#2

This changeset looks good to me 👍

Question: the previous changeset also included some PHP Unit tests in wpThemeJson.php. Should we update the tests accordingly?

@onemaggie commented on PR #3788:


2 years ago
#3

yeah it would be good to include, sadly I don't think I have the bandwidth for that before the end of the year

@hellofromTonya commented on PR #3788:


2 years ago
#4

@audrasjb I can take care of backporting the PHPUnit tests if you haven't yet started on it.

@audrasjb commented on PR #3788:


2 years ago
#5

@audrasjb I can take care of backporting the PHPUnit tests if you haven't yet started on it.

Of course, thanks Tonya :)
I tried to find existing tests on Gutenberg repo, but I'm afraid we don't have any phpunit test to backport from Gutenberg, so maybe we'll have to update the tests directly ;)

#6 @hellofromTonya
2 years ago

  • Component changed from General to Themes
  • Keywords needs-unit-tests added
  • Milestone changed from Awaiting Review to 6.2
  • Owner set to hellofromTonya
  • Status changed from new to accepted

Self-assigning to add unit test(s) and commit candidate review.

@onemaggie commented on PR #3788:


2 years ago
#7

No, this particular change doesn't have unit tests and I agree that it should. If no one gets to them before I'm back I'm happy to work on them.

@hellofromTonya commented on PR #3788:


2 years ago
#8

I'm working on the tests now.

@onemaggie commented on PR #3788:


2 years ago
#9

I'm working on the tests now.

Thanks so much @hellofromtonya

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


2 years ago
#10

  • Keywords has-unit-tests added; needs-unit-tests removed

Refresh of PR #3788: adds test commit (that fails before the backport) and then adds the backport commit, while ensuring it's based on trunk.

This PR backports the changes in #43526 that add support for the outline property on theme.json

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

Props: @MaggieCabrera and @audrasjb (from the previous PR)

@hellofromTonya commented on PR #3788:


2 years ago
#11

hey @MaggieCabrera, I didn't see trunk branch in your forked repo. That made me nervous. So I opened a new PR https://github.com/WordPress/wordpress-develop/pull/3790 which adds the unit test first (which fails) and then adds your fix from the backport (and then test passes).

I'll close this PR in favor of the new one.

#12 @hellofromTonya
2 years ago

  • Keywords has-testing-info added

Test Instructions

Steps to Test

  1. Add the changes to TT3's theme.json per instructions here.
  2. Create a new post.
  3. Add a button block.
  4. Publish the post.
  5. View on the frontend.
  6. Using Dev Tools in your browser, inspect the button's CSS.

Expected Results

Visually:

  • The button should have a red dashed outline around it.
  • When hovering over the button, the outline should change to a solid blue line.

The button's CSS should contain the following properties within .wp-element-button, .wp-block-button__link:

outline-color: red;
outline-offset: 3px;
outline-style: dashed;
outline-width: 3px;

and if you force :hover in the browser's Styles UI, the properties for .wp-element-button:hover, .wp-block-button__link:hover should include:

outline-color: blue;
outline-offset: 3px;
outline-style: solid;
outline-width: 3px;

@hellofromTonya
2 years ago

Test Report results: worked as expected in Chrome

#13 @hellofromTonya
2 years ago

Test Report

Env

  • OS: macOS
  • Browser: Chrome, Firefox, Edge
  • localhost: WP Local and wp-env
  • Theme: TT3
  • Plugins: none activated

Results

See it in action

  • Visually works as expected ✅
  • CSS style properties are present as expected ✅
Last edited 2 years ago by hellofromTonya (previous) (diff)

#14 @ironprogrammer
2 years ago

Test Report

Patch tested: https://github.com/WordPress/wordpress-develop/pull/3790.diff 👍🏻 LGTM!

How great to have a backport so early in the cycle! 🙌🏻

Environment

  • Hardware: MacBook Pro Apple M1 Pro
  • OS: macOS 12.6.1
  • Browser: Safari 16.1
  • Server: nginx/1.23.2
  • PHP: 7.4.33
  • WordPress: 6.2-alpha-54642-src
  • Theme: twentytwentythree v1.0
  • Active Plugins:
    • gutenberg v14.8.0-rc.1 (tested both on 🟢 and off 🔴)

Actual Results

  • ✅ Before patch, outline rules applied to buttons in theme.json have no effect (expected).
  • ✅ After patch, outline and hover style appear as expected in both editor and frontend.

Additional Notes

Supplemental Artifacts

Before: Button blocks with "fill" and with "outline" styles -- outline in theme.json has no effect:
https://cldup.com/L8mXiQFiJb.png

After patch: Button outline styles applied in theme.json appear as expected:
https://cldup.com/5WwOd3mnG3.gif

@ironprogrammer
2 years ago

TT3 modification to add outline and hover styles to button element

#15 @hellofromTonya
2 years ago

  • Keywords commit added

Prepping commit now.

@hellofromTonya commented on PR #3790:


2 years ago
#16

Thanks @audrasjb! Prepping commit right now.

#17 @hellofromTonya
2 years ago

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

In 55008:

Themes: Adds outline CSS properties support in theme.json.

Adds the ability to define outline CSS properties for elements and blocks within theme.json to render outline-color, outline-offset, outline-style, and outline-width styles.

Originally developed and tested in Gutenberg PR 43526.

Props onemaggie, hellofromTonya, audrasjb, ironprogrammer.
Fixes #57354.

#19 @hellofromTonya
2 years ago

Thank you everyone for your contributions!

#20 follow-up: @audrasjb
2 years ago

@ironprogrammer I think it would be better to address TT3 changes in a specific ticket :)

#21 in reply to: ↑ 20 @ironprogrammer
2 years ago

Replying to audrasjb:

@ironprogrammer I think it would be better to address TT3 changes in a specific ticket :)

The attachment was a sample patch to test this ticket on trunk, not a proposed change to TT3 itself 👍🏻I debated putting it into a gist vs attaching the .diff, so I guess I made the wrong choice! 😂

I could have made the attachment comment clearer, not just the filename.

Last edited 2 years ago by ironprogrammer (previous) (diff)

#22 @audrasjb
2 years ago

hahaha ok, thanks for clarifying :D

@onemaggie commented on PR #3788:


2 years ago
#23

hey @MaggieCabrera, I didn't see trunk branch in your forked repo. That made me nervous. So I opened a new PR #3790 which adds the unit test first (which fails) and then adds your fix from the backport (and then test passes).

I'll close this PR in favor of the new one.

oh, that's weird! Thanks for picking this one up Tonya!

#24 @SergeyBiryukov
2 years ago

In 55009:

Themes: Alphabetize the properties list in WP_Theme_JSON::VALID_STYLES for consistency.

Follow-up to [53129], [54253], [55008].

See #57354.

#25 @hellofromTonya
22 months ago

  • Description modified (diff)
  • Summary changed from Backport: outline support for blocks via theme.json to Add outline support for blocks via theme.json

#26 @hellofromTonya
22 months ago

  • Keywords gutenberg-merg added

Adding the experimental keyword for tracking code copied from Gutenberg into Core.

#27 @milana_cap
22 months ago

@hellofromTonya, there's a typo in the gutenberg-merg label.

#28 @ironprogrammer
22 months ago

  • Keywords gutenberg-merge added; gutenberg-merg removed

Corrected typo noted in comment:27 -- thanks, @milana_cap!

Note: See TracTickets for help on using tickets.