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

Patterns: Remove the ref from pattern content #550

Merged
merged 2 commits into from
Jan 27, 2023

Conversation

ryelle
Copy link
Contributor

@ryelle ryelle commented Jan 24, 2023

Fixes #545 — This removes the Navigation's ref property from patterns on output, so that it's never added in the copyable pattern code. The ref is an ID for the nav menu, which only exists on the local site. When added to a user's site, it errors. There should be no ref so that the default behavior takes over when it's added.

Patterns should never have a ref, but in some rare cases they do— patterns created by users with more permissions on the w.org/patterns site, not using the pattern creator, or patterns copied in from external sites.

I decided to fix this by filtering out the ref on the output, rather than just updating the pattern content - this way if someone updates these patterns again or adds new patterns, they'll never have a ref value.

The following is a complete list of published patterns with a ref.

Screenshots

The pattern now correctly displays the fallback (mocked) nav menu:

Before After
Screenshot 2023-01-24 at 10 44 58 AM Screenshot 2023-01-24 at 10 33 00 AM

and when you copy the code, there is no ref.

<!-- wp:group {"align":"full","style":{"spacing":{"padding":{"top":"2em","bottom":"4em"}}}} -->
<div class="wp-block-group alignfull" style="padding-top:2em;padding-bottom:4em"><!-- wp:site-logo {"align":"center"} /-->

<!-- wp:site-title {"textAlign":"center","fontSize":"large"} /-->

<!-- wp:navigation {"layout":{"type":"flex","justifyContent":"center"}} /--></div>
<!-- /wp:group -->

How to test the changes in this Pull Request:

This is easiest to test on a wporg sandbox, since it already has the ref'd patterns. Otherwise create some patterns in wp-admin with Navigation blocks so that they have local ref.

  1. View a pattern with a ref, like Centered header with logo
  2. There should be a visible menu, with "Home About Contact"
  3. Copy the pattern, look at the wp:navigation code, it should not include the ref
  4. Get the pattern via the API, ex https://api.wordpress.org/patterns/1.0/?slug=centered-header-with-logo
  5. Look at the pattern_content value — this is what the core editor uses to display patterns
  6. The wp:navigation code should not have a ref

Removing the ref from navigation blocks before they're rendered so that the block always falls back to the mocked nav menu.
This prevents the ref from appearing in the copyable code.
@ryelle ryelle added the [Component] Pattern Directory The backend of the pattern directory: submission, management, etc label Jan 24, 2023
@ryelle ryelle self-assigned this Jan 24, 2023
@StevenDufresne
Copy link
Collaborator

I tested this locally, it removed the ref and rendered the fallback properly.

I was left with an empty props object, although it didn't cause any issues:
<!-- wp:navigation {} /-->

@tellyworth
Copy link

What's the risk that render_block_data breaks in future, or runs in a different way? Maybe not worth worrying about if the consequences are minor and will quickly be discovered.

@ryelle
Copy link
Contributor Author

ryelle commented Jan 26, 2023

I think it's pretty safe, it's been in core since 5.1 with almost no change, and if something like the format of the block object changes it would be a big deal (for everyone). If it does break, the only thing that would be affected is the live preview on the Pattern Directory, nothing in the API or copyable code.

@ryelle ryelle merged commit a28ff10 into trunk Jan 27, 2023
@ryelle ryelle deleted the fix/remove-ref-from-patterns branch January 27, 2023 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Component] Pattern Directory The backend of the pattern directory: submission, management, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Patterns using a Navigation block have ref set
3 participants