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

Fix wp 6.4/6.3 compat for navigation link variations #59126

Conversation

gaambo
Copy link
Contributor

@gaambo gaambo commented Feb 16, 2024

What?

This is a follow up to #58389. It introduced backwards compatibility for the navigation link variations changes for wordpress 6.4/6.3.
As @jsnajdr found out, the function name used for the variation_callback was wrong (the unprefixed/unbuilt version). And therefore lead to e2e tests expecting the "Page Link" variation to exist failing.

Why?

How?

Change function to correct name. Also removed a wrong @uses comment missed in the last commit.

Testing Instructions

  1. Use WordPress 6.4/6.3
  2. Go into site-editor and add a navigation block
  3. try to search for a block named like "Page Link" or "Post Link"
  4. Block is available

Testing Instructions for Keyboard

Screenshots or screencast

Copy link

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: gaambo <gaambo@git.wordpress.org>

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

@@ -11,7 +11,7 @@ function gutenberg_navigation_link_variations_compat( $args ) {
if ( 'core/navigation-link' !== $args['name'] || ! empty( $args['variation_callback'] ) ) {
return $args;
}
$args['variation_callback'] = 'build_navigation_link_block_variations';
$args['variation_callback'] = 'gutenberg_block_core_navigation_link_build_variations';
Copy link
Member

Choose a reason for hiding this comment

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

Should there really be the gutenberg_ prefix here? I only see a function called block_core_navigation_link_build_variations, without the prefix. Here:

https://github.com/WordPress/gutenberg/blob/trunk/packages/block-library/src/navigation-link/index.php#L354

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's my understading that the code in the lib/compat folder only gets loaded if the gutenberg plugin is active on a wordpress installtion with version 6.4/6.3. But the code is not merged into core. And the gutenber plugin prefixes all functions in the block-library with 'gutenberg_' on build process, to avoid a clash with existinf core functions.
In my local testing with wp 6.4 this works and the function is called -> the variations for core post types and post types registeres before init#10 are created.

Copy link
Member

Choose a reason for hiding this comment

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

You're right, I didn't know this 🙂

@skorasaurus skorasaurus added the [Block] Navigation Link Affects the Navigation Link Block label Feb 16, 2024
Copy link
Member

@jsnajdr jsnajdr left a comment

Choose a reason for hiding this comment

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

👍

@jsnajdr jsnajdr added the [Type] Bug An existing feature does not function as intended label Feb 16, 2024
@jsnajdr jsnajdr merged commit a2d2c2b into WordPress:trunk Feb 16, 2024
61 of 67 checks passed
@github-actions github-actions bot added this to the Gutenberg 17.8 milestone Feb 16, 2024
@youknowriad youknowriad added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Feb 20, 2024
getdave pushed a commit that referenced this pull request Feb 20, 2024
Co-authored-by: gaambo <gaambo@git.wordpress.org>
@getdave
Copy link
Contributor

getdave commented Feb 20, 2024

I just cherry-picked this PR to the cherry-pick-beta-2 branch to get it included in the next release: 9cf215c

@getdave getdave removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Feb 20, 2024
youknowriad pushed a commit that referenced this pull request Feb 20, 2024
Co-authored-by: gaambo <gaambo@git.wordpress.org>
@getdave
Copy link
Contributor

getdave commented Feb 20, 2024

Great work here 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Link Affects the Navigation Link Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants