Skip to content

Conversation

felixarntz
Copy link
Member

@felixarntz felixarntz commented May 28, 2025

Summary

See #1997: This adds basic settings to control the theme support arguments for the View Transitions plugin.

Relevant technical choices

  • Adds settings UI under Settings > Reading for View Transitions theme support arguments:
    • Animation for the default transition type.
    • Selectors for the default configured view transition names.
  • Each setting is optional and merely for customization. The controls allow modifying what would otherwise require to change code, via add_theme_support( 'view-transitions', $args ).
  • The settings are applied to the theme support arguments, overriding / merged into whatever is configured via code.
  • A "Settings" link pointing to the relevant UI is shown in the plugins list table.
  • Minor tweak: The ! defined( 'ABSPATH' ) check was added to all plugin files where it was missing.

Note: Most of the code is inspired by the relatively similar settings implementation for the Speculative Loading plugin. In fact, I copied that code over as a starting point.

Settings UI Screenshot

Screenshot 2025-05-28 1 26 06 PM

@felixarntz felixarntz added [Type] Enhancement A suggestion for improvement of an existing feature no milestone PRs that do not have a defined milestone for release [Plugin] View Transitions Issues for the View Transitions plugin labels May 28, 2025
Copy link

github-actions bot commented May 28, 2025

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: felixarntz <flixos90@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: mukeshpanchal27 <mukesh27@git.wordpress.org>
Co-authored-by: swissspidy <swissspidy@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link

codecov bot commented May 28, 2025

Codecov Report

Attention: Patch coverage is 0.44643% with 223 lines in your changes missing coverage. Please review.

Project coverage is 68.05%. Comparing base (79fd9c0) to head (95d0b4d).
Report is 16 commits behind head on trunk.

Files with missing lines Patch % Lines
plugins/view-transitions/includes/settings.php 0.00% 203 Missing ⚠️
plugins/view-transitions/uninstall.php 0.00% 16 Missing ⚠️
plugins/view-transitions/hooks.php 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk    #2025      +/-   ##
==========================================
- Coverage   70.01%   68.05%   -1.96%     
==========================================
  Files          91       92       +1     
  Lines        7603     7622      +19     
==========================================
- Hits         5323     5187     -136     
- Misses       2280     2435     +155     
Flag Coverage Δ
multisite 68.05% <0.44%> (-1.96%) ⬇️
single 36.99% <0.00%> (-2.61%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@felixarntz felixarntz added this to the view-transitions 1.0.0 milestone May 28, 2025
@felixarntz felixarntz removed the no milestone PRs that do not have a defined milestone for release label May 28, 2025
function plvt_render_settings_field( array $args ): void {
$option = plvt_get_stored_setting_value();

switch ( $args['field'] ) {
Copy link
Member

Choose a reason for hiding this comment

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

There's also no guarantee that $args['field'] is defined. If not set, perhaps this should short-circuit if the expected array keys aren't defined?

Suggested change
switch ( $args['field'] ) {
if ( ! isset( $args['field'], $args['label_for'], $args['field'], $args['description'] ) ) {
return;
}
switch ( $args['field'] ) {

Granted, this is highly unlikely.

Copy link
Member Author

Choose a reason for hiding this comment

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

There's no guarantee from a PHP perspective, but isn't it required at least from a PHPStan perspective due to how the @param is defined? Also it's worth noting this is a private function, so that plus the PHPStan annotation might make this unnecessary.

On the other hand, of course it's good for defensive coding. In that case though, I think we should only bail for field and label_for missing, since description is optional in the first place - if it's not set, we should just not render a description.

Co-authored-by: Weston Ruter <westonruter@google.com>
Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

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

LGTM

@felixarntz felixarntz merged commit de82990 into trunk Jun 2, 2025
20 checks passed
@felixarntz felixarntz deleted the enhance/view-transitions-settings branch June 2, 2025 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Plugin] View Transitions Issues for the View Transitions plugin [Type] Enhancement A suggestion for improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants