-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Remove deprecated behaviors
syntax
#57165
Conversation
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/blocks.php ❔ lib/class-wp-theme-json-gutenberg.php ❔ lib/load.php |
There is no need to sync anything to WordPress Core. The removed/ updated files were never part of Core in the first place because Behaviors didn't ship in 6.4. |
For reference, the backwards compatibility was added in #54071 |
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.
This looks good to me and appears to get rid of the backwards compatibility that should be removed.
However, I'll note that the following testing instructions are incorrect:
- Try to insert the block that contains the removed syntax and it should NOT work anymore:
<!-- wp:image {"behaviors":{"lightbox":{"enabled":true}},"id":157,"sizeSlug":"large","linkDestination":"none"} --> <figure class="wp-block-image size-large"><img src="http://gutenberg.local/wp-content/uploads/2023/11/alan-hardman-SU1LFoeEUkk-unsplash-1024x678.jpg" alt="" class="wp-image-157"/></figure> <!-- /wp:image -->
At the block level, the behaviors
syntax is still supported and migrated via the image's deprecated.js, logic which I believe should still remain intact.
Rather than handling that scenario, this PR removes the support for any custom block that had added the behaviors
block support. It also removes the backwards compatibility for anyone who had enabled the lightbox using the prior version of the UI in the Site Editor.
With that caveat in mind, I think this PR is good to go @michalczaplinski 👍
Thanks for the review, @artemiomorales As far as I can tell, the output_904f74.mp4So, the block migration does not really do anything useful at the moment. It probably could just be removed as well. What do you think? |
@michalczaplinski Hmm, so I believe the migration code doesn't actually run until you make a modification to the post before pressing Update. If you do that, you'll see that the behaviors-deprecation.mp4With that in mind, the code in That being said, I initially thought we would need to keep this to prevent breakage and inconsistencies in folks' block versions; as I'm looking at the deprecation section of the handbook though, I think you're right and we may be able to remove the v8 code from What do you think? I could look at this more closely tomorrow. |
That's weird! I didn't realize that the migration does not run until the post Update. I thought it should have been running on the initial publish as well. I think that If we're removing the compat layer, then we can also remove the deprecations but I don't know if there's a precedent for that. @ajlende Does that sound fine for you? |
Ok pinging @youknowriad @oandregal for feedback. Should we remove the v8 version in the image's |
It runs on
If there's a scenario where old attributes/markup could have been saved and those old attributes/markup are no longer valid, then you need a deprecation. |
Hi, I haven't worked much with deprecations, but this is my understanding:
If we remove the deprecation, what would happen is that old blocks that used the What's important is that changing implementations should keep things working for the user. Ideally, we should maintain the deprecation and use it to migrate the block attributes to the new implementation. An example of the ideal scenario:
Sorry I cannot provide more a more concrete answer. Hope this helps! |
Ok got it! Looks like the deprecation stays @michalczaplinski 👍 |
Thanks for your input folks! I'm merging it as is in that case (keeping the deprecation). |
✅ I updated the PHP Sync Tracking Issue for WP 6.5 to note this PR does not require a backport for WP 6.5. This is due to this comment (in the PR description):
|
What?
Removes the deprecated
behaviors
syntax.Behaviors UI was originally introduced in #49972. It was meant to be a way to add reusable pieces of interactivity to the frontend of sites.
That implementation was removed in #53851 and #54509 and never merged into Core. Backwards-compatibility layer was added for users of the GB plugin.
Why?
It should have already been deprecated in 17.0 because we only offer backwards-compatibility for 2 releases 🙂.
How?
Removing 2 files and references to
behaviors
andlegacyLightboxSettings
.Testing Instructions