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

Sync shadow presets support in theme.json #3915

Closed

Conversation

@Mamaduka
Copy link
Member Author

I just pushed the theme.json update for shadow presets. Based on the diff, this is the only change since WP 6.1, so I won't create a separate PR.

Cc @oandregal

@oandregal
Copy link
Member

I just pushed the theme.json update for shadow presets. Based on the diff, this is the only change since WP 6.1, so I won't create a separate PR.

👍 Good call, those changes pertain to this feature as well.

Copy link
Member

@oandregal oandregal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look good, it has tests, and I verified that it works as expected following the test instructions at WordPress/gutenberg#46813

@hellofromtonya
Copy link
Contributor

Current reviewing. There are a few tweaks needed to the tests. And there's a merge conflict.

@hellofromtonya hellofromtonya force-pushed the sync/shadow-presets-support branch from fec0a49 to 020373c Compare February 1, 2023 17:14
@hellofromtonya
Copy link
Contributor

Rebase this PR on top of wordpress-develop current trunk branch to resolve the merge conflict. Force pushed.

Comment on lines 4463 to 4464
$this->assertEquals( $styles, $theme_json->get_stylesheet(), 'Returned of "::get_stylesheet" does not match expectations' );
$this->assertEquals( $styles, $theme_json->get_stylesheet( array( 'variables' ) ), 'Returned of "::get_stylesheet" does not match expectations' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • use assertSame()
  • Failure messages need to be different to identify which assertion failed.

Commit coming.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in 5e3e536.

$element_styles = 'a:where(:not(.wp-element-button)){box-shadow: var(--wp--preset--shadow--natural);}.wp-element-button, .wp-block-button__link{box-shadow: var(--wp--preset--shadow--natural);}p{box-shadow: var(--wp--preset--shadow--natural);}';
$styles = $global_styles . $element_styles;

$this->assertEquals( $styles, $theme_json->get_stylesheet() );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • use assertSame()

Commit coming.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in 5e3e536.

Copy link
Contributor

@hellofromtonya hellofromtonya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Confirmed both GB PRs are present in this PR ✅
  • Changes are noted in @since 6.2.0
  • Includes tests ✅
  • Has a test report ✅

Ready for commit.

@hellofromtonya
Copy link
Contributor

@Mamaduka Mamaduka deleted the sync/shadow-presets-support branch May 9, 2023 09:19
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