-
Notifications
You must be signed in to change notification settings - Fork 128
Inform the user if the current theme explicitly supports view transitions with its own configuration, and add a UI control to make overriding that configuration via settings optional #2037
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
Conversation
…ions with its own configuration, and add a UI control to make overriding that configuration via settings optional.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #2037 +/- ##
==========================================
- Coverage 67.74% 67.31% -0.44%
==========================================
Files 93 93
Lines 7664 7718 +54
==========================================
+ Hits 5192 5195 +3
- Misses 2472 2523 +51
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @felixarntz for the PR!
The admin transition currently doesn’t respect the override setting, which needs to be addressed. Other than that, I’ve left some feedback comments.
add_action(
'after_setup_theme',
static function () {
add_theme_support( 'view-transitions', array( 'default-animation' => 'slide-from-right', 'enable_admin_transitions' => false ) );
}
);
I don't think the admin transition needs to respect it, as the admin view transition behavior is independent of the theme's. So the checkbox to enable view transitions in WP Admin should always apply, regardless of what the theme does. Is that not the case currently? |
Yes, it is currently the case. However, for better clarity, if this feature is meant to be independent, we should consider mentioning that either in a notice or somewhere else in the UI so the end user is aware. |
Would it make sense to have that checkbox in an entirely separate section on the page? That would make it clear that it's not subject to the same notice. For example, in addition to the "View Transitions" section, we could have an "Admin View Transitions" section. |
+1 |
@mukeshpanchal27 I implemented a separate settings section for WP Admin in 469b8f0, and also addressed your other feedback. This is ready for another review. |
Summary
This solves the problem that currently the settings will always be applied, even if the current theme already supports view transitions explicitly. At the moment, this means the actual theme support arguments are never respected, as the plugin will always apply the settings to it.
Relevant technical choices
Testing
To test the different behavior when using a theme with explicit support, the active theme must
add_theme_support( 'view-transitions', $args )
, with a concrete list of arguments, in other words, not justadd_theme_support( 'view-transitions' )
.For example, add this to your theme's
functions.php
:With this added, your site should now always use the "slide from right" animation for view transitions, even if the WP Admin setting for the default transition animation is set to something else (e.g. the default "fade"). Only if you activate the new checkbox to override the theme's config, the other WP Admin settings for the frontend will take effect, overriding whatever the theme defines.