Skip to content

Backport: outline support for blocks via theme.json #3788

Closed
MaggieCabrera wants to merge 1 commit into
WordPress:trunkfrom
MaggieCabrera:backport-43526
Closed

Backport: outline support for blocks via theme.json #3788
MaggieCabrera wants to merge 1 commit into
WordPress:trunkfrom
MaggieCabrera:backport-43526

Conversation

@MaggieCabrera

@MaggieCabrera MaggieCabrera commented Dec 19, 2022

Copy link
Copy Markdown

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


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@audrasjb

Copy link
Copy Markdown
Contributor

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?

@MaggieCabrera

Copy link
Copy Markdown
Author

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

Copy link
Copy Markdown
Contributor

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

@audrasjb

Copy link
Copy Markdown
Contributor

@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 ;)

@MaggieCabrera

Copy link
Copy Markdown
Author

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

Copy link
Copy Markdown
Contributor

I'm working on the tests now.

@MaggieCabrera

Copy link
Copy Markdown
Author

I'm working on the tests now.

Thanks so much @hellofromtonya

@hellofromtonya

Copy link
Copy Markdown
Contributor

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.

@MaggieCabrera

Copy link
Copy Markdown
Author

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants