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

Synced blocks: Define them as such in the Inspector #43261

Open
jameskoster opened this issue Aug 16, 2022 · 18 comments
Open

Synced blocks: Define them as such in the Inspector #43261

jameskoster opened this issue Aug 16, 2022 · 18 comments
Assignees
Labels
[Block] Template Part Affects the Template Parts Block [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) Needs Dev Ready for, and needs developer efforts [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@jameskoster
Copy link
Contributor

Related to #42482 and #32163.

When you select a template part or reusable block, it might be nice to clarify their global / synced nature – and perhaps outline usage – in the Inspector.

@jameskoster jameskoster added [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) [Block] Template Part Affects the Template Parts Block labels Aug 16, 2022
@jameskoster
Copy link
Contributor Author

A rough idea:

Screenshot 2022-08-16 at 11 02 53

Perhaps clicking the pill could open a modal that explains the concept in mode detail:

Screenshot 2022-08-16 at 11 04 01

@jameskoster jameskoster added the Needs Design Feedback Needs general design feedback. label Aug 16, 2022
@carolinan
Copy link
Contributor

I am hesitant about introducing yet another term, "Synced blocks". Can the features of these blocks be described without it?
I feel like "Used in 4 X" Where the X represents the actual item -posts, page, template etc, would be easier to understand?

We should consider that this is also true for the site title, tagline, logo/site icon.

@jameskoster
Copy link
Contributor Author

I'm sure there's room to explore the copy! It might be tricky to use absolute terms though, since things like Reusable Blocks might appear in posts, pages, templates, template parts... the string might get a bit long 😅

We should consider that this is also true for the site title, tagline, logo/site icon.

Good point. Navigation too.

@mtias
Copy link
Member

mtias commented Sep 10, 2022

Inferring how much a block is used throughout a site is not a trivial task, so I'd suggest a description that avoid saying that.

@jameskoster
Copy link
Contributor Author

Since synced blocks are posts in their own right, could basic usage be saved as meta data to make it more trivial?

If not, it would be nice to do something to differentiate between local/global blocks, as it's a pretty big pain point right now. It could be something like this, or #32163, or a combination :)

@mtias
Copy link
Member

mtias commented Sep 12, 2022

Not really, the problem is figuring out how to keep it updated. Every time you embed a reusable block you'd bump the usage? What happens when it gets removed? What if you duplicate a post, or unpublish a page, or remove a template that referenced one? It gets unwieldy quickly. The right way to do it would be to query for its presence with has_blocks, which would need to be done performantly and on some cron job. Not a trivial thing to design well.

I don't think it adds that much context, anyways. A simpler "This block is synced" seems it fulfills mostly the same purpose.

@jameskoster
Copy link
Contributor Author

Every time you embed a reusable block you'd bump the usage?

I was thinking the 'count' would be updated when the parent document is updated / published. But yes I suppose that will be tricky considering imports etc.

A simpler "This block is synced" seems it fulfills mostly the same purpose.

I agree, this would be a worthwhile update on its own.

@getdave
Copy link
Contributor

getdave commented Dec 8, 2022

So I took a look at this in the context of the Nav block. I realised that a few places in the codebase define isSynced as being isReusableBlock or isTemplatePart. I'm wondering if a better proxy might be areInnerBlocksControlled from @wordpress/block-editor?

That way it could apply generically to any block which is sync'd with an entity.

@getdave
Copy link
Contributor

getdave commented Dec 8, 2022

@jameskoster
Copy link
Contributor Author

Thanks for taking a look.

This is where the fundamentally different behaviour of the Navigation and Template Part blocks makes things a bit awkward.

With template parts (and reusable blocks), their appearance in the UI is informed by the referenced content. IE when I insert my "Centered Header" template part, that's how I see it presented in the toolbar, list view, etc:

Screenshot 2022-12-09 at 12 48 43

The template part 'wrapper' is obfuscated away, so in this context it makes sense to mark the blocks as synced.

As you know Navigation doesn't work like this. It's always presented as "Navigation", regardless of which menu is referenced. This makes me question whether we should mark the Navigation as synced, because it's not really true. It's the referenced menu that is synced, not the parent block. I worry it might be confusing to have multiple Navigation instances in a document that are all marked as synced despite having (potentially) different appearances.

If Navigation behaved like Template Parts this wouldn't be an issue, but I appreciate that will be a big can of worms :)

@scruffian
Copy link
Contributor

If Navigation behaved like Template Parts this wouldn't be an issue, but I appreciate that will be a big can of worms :)

What changes would you expect to see in the navigation block for it behave like a Template Part? Here are some I thought of:

  1. Display the name of the selected navigation menu (if there is one) in the block toolbar
  2. Do the same thing in the list view

What else am I missing?

@jameskoster
Copy link
Contributor Author

jameskoster commented Dec 9, 2022

I would consider the block name in the Inspector too, but that might be trickier for Navigation. Worth noting this doesn't currently happen for template parts which is a bug to fix (#42154).

If there's a plan to support navigation editing in focus mode, then might be worth keeping an eye on too.

Edit: I forgot one, template parts and reusable blocks appear in the Inserter by name:

Screenshot 2022-12-09 at 17 21 26

@talldan
Copy link
Contributor

talldan commented Dec 12, 2022

I've made a PR that shows the menu name in some of those areas - #46457.

It also fixes #42154 (I can extract that out to a separate patch if needed).

I forgot one, template parts and reusable blocks appear in the Inserter by name:

That should be possible too, though some users may have a lot of menus. Happy to try it out in a another PR, though I don't think it'll be quite as easy as it was for template parts.

@getdave
Copy link
Contributor

getdave commented Dec 12, 2022

Thanks for the feedback folks. I'm glad the PR has sparked discussion.

Perhaps I should ask a different question? What can we do design wise to make it more obvious that the contents of a given Navigation block are synced to an entity?

The requirement for improvement is there and now we need to devise a solution.

I don't think we necessarily have to make it identical to how TPs or RBs appear. But some of the styling could be similar.

@scruffian has made some suggestions.

@SaxonF @jasmussen may also have a view?

@jasmussen
Copy link
Contributor

I think #46438 helps, #46457 can as well though to a lesser extent. As Jay suggests, should we also apply the purple "synced" color? (Both for any clickthrough overlays applied, but also to the icon and select state in the inspector). Those would be some initial steps, and I imagine more pieces of the puzzle could fall into place as the inspector editing interface matures.

@getdave
Copy link
Contributor

getdave commented Dec 12, 2022

As Jay suggests, should we also apply the purple "synced" color? (Both for any clickthrough overlays applied, but also to the icon and select state in the inspector).

That's doable. I can look into that. Still open to any additional thoughts as well.

Thank you to everyone who contributed here. It's much appreciated.

@jordesign
Copy link
Contributor

Wondering if the way we present Synced Patterns in 6.3 and onwards is sufficient to close this issue @jameskoster ?

Screenshot 2023-09-04 at 11 48 01 am

@jordesign jordesign added the [Type] Enhancement A suggestion for improvement. label Sep 4, 2023
@jameskoster
Copy link
Contributor Author

jameskoster commented Sep 4, 2023

I don't think so because template parts are not captured.

At some point we'll probably want to allow custom descriptions for synced patterns, so the pill still seems like the best approach to me. Perhaps with "Syncing: Fully synced" to echo the details panel:

Screenshot 2023-09-04 at 11 19 13

@jameskoster jameskoster added Needs Dev Ready for, and needs developer efforts and removed Needs Design Feedback Needs general design feedback. labels Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Template Part Affects the Template Parts Block [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) Needs Dev Ready for, and needs developer efforts [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants