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

The pattern shuffle feature includes synced patterns but they are inserted unsynced #62294

Closed
ndiego opened this issue Jun 4, 2024 · 6 comments · Fixed by #62422
Closed

The pattern shuffle feature includes synced patterns but they are inserted unsynced #62294

ndiego opened this issue Jun 4, 2024 · 6 comments · Fixed by #62422
Assignees
Labels
[Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@ndiego
Copy link
Member

ndiego commented Jun 4, 2024

Description

If you have created a synced pattern and placed it in an existing pattern category, when you insert another unsynced pattern from that category and use the new shuffle feature, the synced pattern is included in the shuffle. However, the inserted synced pattern is no longer synced.

Step-by-step reproduction instructions

  1. Use Gutenberg trunk and TT4
  2. Open a page and insert one of the banner patterns
  3. Modify the pattern slightly (so you can recognize it) save it as a synced pattern and add it to the "Banners" category
  4. Now add another banner pattern, you should see the shuffle button
  5. Shuffle through the patterns until your synced pattern gets inserted. Notice that it's no longer synced.

Screenshots, screen recording, code snippet

shuffle-bug-synced-pattern.mp4

Environment info

  • Gutenberg trunk
  • WordPress 6.5

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@ndiego ndiego added [Type] Bug An existing feature does not function as intended [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced labels Jun 4, 2024
@ndiego ndiego moved this to ❓ Triage in WordPress 6.6 Editor Tasks Jun 4, 2024
@fabiankaegy fabiankaegy moved this from ❓ Triage to 📥 Todo in WordPress 6.6 Editor Tasks Jun 4, 2024
@richtabor
Copy link
Member

It's technically working as expected: shuffling among the pattern within the inserted category.

As it shuffles to another pattern, the alternate pattern could be any pattern in that same category—not necessarily any other synced pattern. You're not editing the synced pattern when shuffling.

@ndiego
Copy link
Member Author

ndiego commented Jun 4, 2024

As it shuffles to another pattern, the alternate pattern could be any pattern in that same category—not necessarily any other synced pattern. You're not editing the synced pattern when shuffling.

No, I get that, but it inserts the synced pattern as an unsynced pattern. Is that correct?

@talldan
Copy link
Contributor

talldan commented Jun 6, 2024

No, I get that, but it inserts the synced pattern as an unsynced pattern. Is that correct?

I agree with you @ndiego, it doesn't sound correct to me. Synced patterns should probably not be considered as candidates for shuffling, given it's unexpected that they'll be inserted as unsynced.

If they were inserted as synced the shuffle option would disappear as synced patterns don't support it at the moment.

@jorgefilipecosta jorgefilipecosta self-assigned this Jun 6, 2024
@jorgefilipecosta
Copy link
Member

If they were inserted as synced the shuffle option would disappear as synced patterns don't support it at the moment.

Hi @talldan, do you think it would make sense to make synced patterns also support shuffling?

@richtabor
Copy link
Member

Synced patterns should probably not be considered as candidates for shuffling, given it's unexpected that they'll be inserted as unsynced.

If they were inserted as synced the shuffle option would disappear as synced patterns don't support it at the moment.

This makes sense to me.

@talldan
Copy link
Contributor

talldan commented Jun 10, 2024

Hi @talldan, do you think it would make sense to make synced patterns also support shuffling?

I haven't been involved in the recent shuffling work much, so I don't know if it has been discussed before. (@scruffian might be a good person to comment).

Synced patterns are all user created right now (though it could change in the future - #59272), so if the user has sensible categories it makes sense to me that they can leverage shuffle.

Overrides mean that pattern content can be edited, so #62288 is still a concern for synced patterns. It would be possible to maintain the user edits when shuffling synced patterns if the new pattern is compatible, but I imagine it'd result in very few matching patterns. Or you could very easily check for edits (the pattern (core/block) block's content attribute is non-empty when there are edits).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants