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 ability to change sync state in UI with a classic theme #51920

Closed
annezazu opened this issue Jun 26, 2023 · 8 comments · Fixed by #51998
Closed

Patterns: remove ability to change sync state in UI with a classic theme #51920

annezazu opened this issue Jun 26, 2023 · 8 comments · Fixed by #51998
Assignees
Labels
[Block] Pattern Affects the Patterns Block [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced REST API Interaction Related to REST API [Type] Bug An existing feature does not function as intended

Comments

@annezazu
Copy link
Contributor

In testing 16.1 RC1 and the new pattern syncing, I noticed that with a classic theme you can control the sync state. In talking with @youknowriad about this he indicated this shouldn't be possible because otherwise one will be able to create "reusable blocks" that references a "non synced pattern".

In this case, we should show the sync state but not allow the option to edit. If it does remain for some reason, the design needs to be updated as the sync toggle isn't descriptive of what's changing, goes to the next line when toggling, and initiates a save state when no changes occur.

classic.theme.save.state.pattern.mov
@annezazu annezazu added [Type] Bug An existing feature does not function as intended REST API Interaction Related to REST API [Block] Pattern Affects the Patterns Block labels Jun 26, 2023
@annezazu annezazu moved this to ❓ Triage in WordPress 6.3.x Editor Tasks Jun 26, 2023
@youknowriad
Copy link
Contributor

We should probably forbid editing that property in the REST API too.

@annezazu annezazu added the [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced label Jun 26, 2023
@ndiego ndiego moved this from ❓ Triage to 📥 Todo in WordPress 6.3.x Editor Tasks Jun 27, 2023
@glendaviesnz
Copy link
Contributor

I am not sure I follow this. A classic theme has the ability to insert both synced and unsynced user-added patterns, so why would we want to stop them from changing the sync status of a given pattern but allow it for a block theme? This same toggle appears for block themes also.

because otherwise one will be able to create "reusable blocks" that references a "non synced pattern".

If somebody creates a 'synced' pattern and inserts some instances of it in posts, and then toggles the source pattern to unsynced all existing instances will remain synced and only new instances will be inserted as unsynced.

@talldan
Copy link
Contributor

talldan commented Jun 27, 2023

If somebody creates a 'synced' pattern and inserts some instances of it in posts, and then toggles the source pattern to unsynced all existing instances will remain synced and only new instances will be inserted as unsynced.

Yeah, so I guess the issue is that you can still edit those synced instances and have unexpected updates propagating.

We did previously have it so that the inner blocks auto-detached themselves when the block detected it was unsynced, but it felt a bit risky.

@glendaviesnz
Copy link
Contributor

glendaviesnz commented Jun 27, 2023

So just to expand on what Dan has said, initially we had it so if a source pattern was toggled from synced to unsynched when an instance of that pattern was opened in the editor it overwrote itself with its innerBlocks in the same what that using the 'Convert to blocks' with a reusable block does.

We thought the risk with this was that one user could toggle to unsynced and another user could edit what they thought was a synced block and wonder why the change then wasn't reflected everywhere. Having the change only applied to newly inserted instances avoids this ... but of course has the reverse risk that the user editing the existing synced block may wonder why the changes are reflected on all the other existing instances when they turned off synced

I think this is a case with no perfect solution. With the initial implementation it felt limiting for the user to have the option to set the sync status when adding, but then no way to update it, but it looks like we need to decide which is the best tradeoff:

  1. Make sync status a one-time decision on creation, with the only option to change for the user to recreate the pattern with a different status. This is more inconvenient if the user changes their mind about the sync status and wants to change it but avoids any confusion about how existing synced instances might behave when the source pattern is toggled to unsycned
  2. Leave it as is with an existing instances remaining synced if the source pattern is toggled to unsynced, unless the user decides to us the 'convert to blocks' option to unsync them
  3. Switch back to our original setup where existing instances of synced patterns automatically unsync themselves if edited after their source pattern is unsynced.

Both options 2 & 3 still leave the potential for some confusion with unedited blocks. A user could toggle to unsynced and then wonder why new changes applied to source pattern are reflected in unedited blocks in the frontend, or toggle to synced and wonder why changes to source pattern are not reflected in previous versions of the block that they inserted.

Any thoughts on the best option for 6.3?

@youknowriad
Copy link
Contributor

Make sync status a one-time decision on creation, with the only option to change for the user to recreate the pattern with a different status. This is more inconvenient if the user changes their mind about the sync status and wants to change it but avoids any confusion about how existing synced instances might behave when the source pattern is toggled to unsycned

For me that's the best option, especially since we also said that potentially we could make the "sync" status per instance later. If a user changes his mind he can copy the content and create a new pattern.

@carolinan
Copy link
Contributor

Having a duplicate or copy option on the pattern management screens would help reduce the inconvenience.

@glendaviesnz
Copy link
Contributor

Having a duplicate or copy option on the pattern management screens would help reduce the inconvenience.

I believe there is an intention to add this, but not sure about the timeline.

@stratboy
Copy link

stratboy commented Oct 2, 2024

I personally would really like to see that feature back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Pattern Affects the Patterns Block [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced REST API Interaction Related to REST API [Type] Bug An existing feature does not function as intended
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

7 participants