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

update safe-svg block apiVersion to version 3 #133

Merged
merged 6 commits into from
Oct 10, 2023
Merged

Conversation

fabiankaegy
Copy link
Member

Description of the Change

WordPress 6.3 adds a new apiVersion which indicates that a block supports being rendered inside an iframed editor.

Closes #

How to test the Change

Changelog Entry

Added - New feature
Changed - Existing functionality
Deprecated - Soon-to-be removed feature
Removed - Feature
Fixed - Bug fix
Security - Vulnerability

Credits

Props @username, @username2, ...

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

WordPress 6.3 adds a new `apiVersion` which indicates that a block supports being rendered inside an iframed editor.
@jeffpaul jeffpaul added this to the 2.2.0 milestone Jul 10, 2023
@jeffpaul jeffpaul requested review from a team and ravinderk and removed request for a team July 10, 2023 21:03
@@ -1,6 +1,6 @@
{
"$schema": "https://schemas.wp.org/trunk/block.json",
"apiVersion": 2,
"apiVersion": 3,
Copy link
Contributor

@ravinderk ravinderk Jul 11, 2023

Choose a reason for hiding this comment

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

@fabiankaegy I approved this pull request, but I want to confirm whether it will work fine with WordPress 5.7 (plugin minimum dependency).

Copy link
Member

Choose a reason for hiding this comment

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

@fabiankaegy bumping this back up for your input

Copy link
Collaborator

@dkotter dkotter Aug 16, 2023

Choose a reason for hiding this comment

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

So based on the documentation, v3 of the API is WP 6.3+, whereas v2 is 5.6+. That said, not clear from that if things won't work at all or if it gracefully falls back to an older API version.

Just tested in a WP 5.7 environment and the SVG block doesn't work, whether using v1, v2 or v3 of the Block API. Existing blocks still render but adding a new SVG block throws an error.

Updated to 5.8 and things worked, both on v2 and seemingly on v3 as well. Not a super thorough test but I think we should look to bump our minimum WordPress version to at least 5.8 since it doesn't seem the block works on 5.7 anyway.

Still defer to @fabiankaegy's opinion on if there's any concern with using API v3 on an environment running a WP version less than 6.3. If no concern, I'd suggest we bump the WP version in this PR. Otherwise that can be done in a separate PR and maybe we close this out until 6.3 is our minimum

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine bumping the WP minimum to 6.1 for that matter, been considering a L-2 support policy for WP so if we bump "tested up to" 6.3 then that would be ok by me.

@dkotter dkotter modified the milestones: 2.2.0, 2.2.1 Aug 17, 2023
@jeffpaul
Copy link
Member

@fabiankaegy is there a reason to keep this in draft or can it move out for review?

@fabiankaegy
Copy link
Member Author

@jeffpaul Sorry for the delay here. No concerns from my end. This is fully backward compatible and we can merge it as is I think :)

@fabiankaegy fabiankaegy marked this pull request as ready for review September 19, 2023 14:29
@fabiankaegy fabiankaegy requested a review from a team as a code owner September 19, 2023 14:29
@jeffpaul jeffpaul merged commit b6b186e into develop Oct 10, 2023
11 checks passed
@jeffpaul jeffpaul deleted the fix/api-version-3 branch October 10, 2023 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants