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

Pattern Overrides: Improve the UI clarity for using named overrides #59813

Closed
talldan opened this issue Mar 13, 2024 · 18 comments
Closed

Pattern Overrides: Improve the UI clarity for using named overrides #59813

talldan opened this issue Mar 13, 2024 · 18 comments
Labels
[Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced Needs Design Needs design efforts. [Type] Enhancement A suggestion for improvement.

Comments

@talldan
Copy link
Contributor

talldan commented Mar 13, 2024

Problem

The PR 'Use block naming for marking blocks as overridable in patterns' introduced a new way to mark a block within a pattern as overridable by naming the block.

A problem is that this piggybacking on the block naming functionality doesn't resulting effect of naming a block isn't clear to the user, there's no mention of pattern overrides anywhere.

Conversely, if the UI is changed, but ends up too geared towards pattern overrides, users might be confused about how to name a block for organizational purposes (see Pattern Overrides: No opt-out mechanism exists for named overrides).

A secondary flow for creating patterns is to create some blocks (e.g. while editing a post), select them and create a pattern from them. The existing names for these blocks will be retained, and could be used for to create pattern overrides, but potentially a user should be aware of the resulting pattern's behavior.

A related problem is making the user aware of the risks of renaming a block. After #59268, the block name is used as the connecting glue for the block bindings feature that drives pattern overrides. It's what's used to reference the content values stores in a pattern instance. If the block name changes, it can result in those content values no longer being referenced correctly.

@talldan talldan added [Type] Enhancement A suggestion for improvement. Needs Design Needs design efforts. [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced labels Mar 13, 2024
@talldan talldan changed the title Pattern Overrides: Improve naming flow for marking blocks within patterns as overridable Pattern Overrides: Improve the UI clarity for using named overrides Mar 13, 2024
@SaxonF
Copy link
Contributor

SaxonF commented Mar 18, 2024

Adding a few different approaches here in case it inspires other ideas.

Button that triggers modal to give the block a name and choose which attributes to override (if we decide to go that granular)

image

Making use of similar pattern to block connections where you can "add" overrides. Naming block can either be a suggestion or if block does not already have a name, prompt for name on first adding override.

image

Toggle that highlights block name when enabled

image

With all these approaches overrides are not tied to name. I'm not convinced doing that is the right approach and would rather see us encourage block naming as part of the experience. We'd need to map name to ID as suggested in previous suggestions.

@fabiankaegy
Copy link
Member

I like the UX of the flow @SaxonF is proposing here. It both uses an opt-in approach and showcases how we could make the overrides more granular in the future.

Something I think we will still need to solve is the issue of renaming side effects. Even a simple change such as changing the capitalization of a block name or fixing a typo will mean that all existing instance overrides have lost their connection to the field. That is a very disruptive action that is very hard for a user to realize because they don't see all the instances.

We should come up with some way of making the user aware of the impact of renaming a field after it allows overrides has.

@SaxonF
Copy link
Contributor

SaxonF commented Mar 21, 2024

There is no technical way we can make it not break the connection when renaming?

@kevin940726
Copy link
Member

There is no technical way we can make it not break the connection when renaming?

If we only use the "metadata.name" attribute, then no, I don't think so. I believe there are some ways to internally store an identifier which doesn't change when we rename the block though, that's the same idea behind the id approach.

@jasmussen
Copy link
Contributor

If renaming in the list view breaks the connection, we should not allow connected blocks to be renamed there. We can consider graying out the "rename" option, perhaps even show help text. But I'd echo Saxon's instincts here: it feels entirely unintuitive that renaming some blocks but not others break block connections.

I can imagine some exotic cases where you have two named blocks, "Title" and "Subtitle", and after you connected those up elsewhere you decide to swap those, i.e. rename "Title" to "Subtitle" and vice versa. Presumably that would "reconnect" with existing block connection pointers, yes?

Building on that, can we know whether a block is connected elsewhere in the UI, and show a modal "confirm" dialog showing those connections breaking when renaming? And if not, then it seems we need some other way to decide whether to show the confirm modal, could be as simple as requiring that confirm modal if Saxon's "allow overrides" toggle is turned on.

@annezazu
Copy link
Contributor

Looking over these early design ideas, I like most the first approach as it feels fairly user friendly. However, reading about the impact of renaming, I wonder if we need to do away with the "naming" word. We have naming in List View and this feels different and, unless I'm mistaken, naming this won't impact list view naming. Even if naming is happening underneath the surface, we don't need to bring that complexity into the interface. Here's an idea:

Screenshot 2024-03-22 at 1 56 18 PM
Screenshot 2024-03-22 at 1 58 30 PM
Screenshot 2024-03-22 at 1 58 19 PM

P.S. Can you add a link to the figma file for this work? I wanted to make a copy of what you did to better explain but, without a link, I can't copy and show the idea readily. I ended up taking a screenshot then writing text overtop haha.

@jasmussen
Copy link
Contributor

Where this gets tricky is that it's the same naming aspect that is used — both for the editorial/convenience naming, and for override naming. So if we actually diverge here I worry it only adds much more confusion. Diving more into this, I keep coming back to:

  • Having a toggle to flip, as Saxon has shown, to make the block use its name to connect
  • Add "are you sure" modals with a description of the impact (potentially breaking a connection) when you try and rename the block after that toggle has been flipped.

@annezazu
Copy link
Contributor

Does this mean that if I generally name a block in List View while creating a pattern, as we recommend folks do, it will automatically turn on overrides? I just want to make sure I'm understanding that correctly as I worry based on the current way renaming blocks has been positioned that this will cause an immense amount of confusion and I'd rather see this as a different mechanism. Perhaps it's just me but I'm struggling to see benefits of it being the same.

@jasmussen
Copy link
Contributor

No, I don't think it should, that's why I would think the toggle is good. But the naming field could/should be the same, so if you already named the block, and then turn on the toggle, you're all set.

I'm working within the boundaries of naming the block as a constraint for this work, and within those boundaries I'm trying to see it from the opposite angle: if we have to name a block, it would arguably be more confusing if there were two ways to name the block, one just cosmetically, the other, functionally. A different comparison is synced blocks, those are named and show up with that name in the list view, in fact they are not possible to rename there. That could be a pattern to follow here too.

@kevin940726
Copy link
Member

There's an explorative PR #60066 that adds a toggle button to allow opting out of pattern overrides. The difference is that it automatically toggles on when naming a block.

@jasmussen
Copy link
Contributor

Just to unstick this and expand a bit on my comment here, here's a quick sketch trying to expand on Saxon's initial concepts. These are essentially the same, give or take some rephrasing, just with a bit more context from the full editor, list view, etc.

A confirm toggle

In this example, I have a synced pattern called "Post with Image", that I'm editing to make it into a partially synced pattern.

Block connections, confirm toggle
  • Clicking the toggle allows that block to be overridden.
  • When toggled, the block icon turns purple, and the block name is pre-filled (unless already filled out) with the block name.
  • If I change that block name, such as in Fig 3. changing it from "Image" to "Hero", a modal will appear on-blur to ask you if you're sure you want to change that block name, explaining what the consequences might be.

It would be very nice if we could, in the future, show that contextual modal only if we knew that instances of the partially synced pattern were actually used and overridden elsewhere. But that's tricky and doesn't seem likely to happen any time soon, if at all.

But this modal would exist to address the concern about losing connections.

I also sketched out a "allow overrides" button instead of the toggle, to go with Saxon's other suggestion. This is less obvious in the broader context, IMO, especially for when you want to disallow overrides again.

Figma.

@annezazu
Copy link
Contributor

Thank you for writing and designing this out! I really dig the approach of the toggle being separated out from naming and still am not sold on calling it naming but that's just me. In many cases, I can see how pulling the name from any custom List View naming would make it an easy workflow (and could see cases were folks might want different names for each or might go to update in list view only to have the override functionality impacted). I'm assuming though that we wouldn't name an override based on just the name of the block as you show with Image? That feels like it would get messy really quick if you had multiple images that could be overridden and you need a unique name. There just seem to be a number of "gotchas" here:

  • If you want a different name for the override than what's in List View.
  • If you want to change the name of the override either under advanced or, I'm assuming, in List View.
  • If you have multiple blocks with overrides that have the same name (no custom name but are the same block type), this might cause weird naming conventions for overrides that could be confusing. Unclear!

This is a reminder for myself too: As much as we can, let's try to think about this feature in the broader context of naming blocks, creating patterns, and using patterns.

@talldan
Copy link
Contributor Author

talldan commented Mar 27, 2024

  • Clicking the toggle allows that block to be overridden.
  • When toggled, the block icon turns purple, and the block name is pre-filled (unless already filled out) with the block name.

So to clarify, if I'm editing a paragraph that doesn't have a name:

  • I toggle overrides on
  • 'Paragraph' appears as the default name (I'm assuming that's what's meant by prefilling with the block name, it's a little confusing that there are two things we refer to as 'block name')
  • I change that to my desired name

A question is whether the modal appears after that first edit? I'd imagine not, but it might be difficult to tell when the first edit happens vs. a later rename. We can try some experiments around this though to see what feels right.

Another small detail is that if you have two paragraphs both named 'Paragraph', they will have a binding to the same data. So if you edit one in an instance the other updates to the same value (it doesn't completely work in trunk, but it's the desired behavior). Having this as the default might cause some confusion. 🤔

My feeling is that it might be good if the user is more guided towards using a meaningful name for the blocks because of the desire for the names to have value as a schema.

I'd appreciate @youknowriad's input here.

Thanks for all the explorations btw, it does feel like we're making progress, it's a tricky thing to get right.

@jasmussen
Copy link
Contributor

If you want a different name for the override than what's in List View.

Should this be allowed? The value proposition here isn't clear to me, vs. the simplicity of only ever having a single way to name block, and having overriding that named block be a separate yes/no choice.

A question is whether the modal appears after that first edit? I'd imagine not, but it might be difficult to tell when the first edit happens vs. a later rename. We can try some experiments around this though to see what feels right.
Another small detail is that if you have two paragraphs both named 'Paragraph', they will have a binding to the same data.

Good observations. Yes, I wonder what makes the most sense here, in case Saxon's other design, a button+modal instead of a toggle solves this better? It could even tie into the fact that renaming in the list view also happens in a modal.

Perhaps the trickiest part of the toggle is the risk of leaving the name empty. I wonder what happens if you clear out the field and blur it, capturing the whole flow in a modal might add guardrails to that.

@jasmussen
Copy link
Contributor

jasmussen commented Apr 1, 2024

Quick visual test to the button approach:

Block connections, button instead

This leans into: there's only one block name, and it's used for overrides as well. And this unifies the name modal.

Figma.

Edit: The "allow overrides" toggle would only ever be available inside a synced pattern.

@gaambo
Copy link
Contributor

gaambo commented Apr 10, 2024

The handling of pattern overrides binding / renaming is spread across a few issues/PRs, so I'm not entirely sure, this is the correct place. And I'm kind of loosing track of the current direction.
But this issue seems to have the most concrete discussion, so I'm chiming in here. It may be stuff already pointed out by others or in other issues, sorry if repeating, but I think getting this right is really important to give patterns the power they can have.

I would want to avoid a solution which is difficult for users/pattern-builders/editors just to work around the fact that we're using the name of the block. Which seems to be the case with some of the UI explorations here: Adding an additional step to opt-out/redo renaming/adding two different kind of names(?).

Maybe the end-goal of user-experience is not clear yet (or everyone has their own 😅), so I'm trying to define my ideal flow here:

  • As a pattern builder/theme developer, I would want overrides to be opt-in
  • As a pattern builder I would like to build patterns and name blocks inside it for clarity for the editor-user/organization purposed, and not automatically enable overrides there.
  • As a pattern builder, I would like the possibility to rename block inside a pattern. Possible reasons: Typos, better organization (because new items are added, things made clearer after initial setup) or a simple one: translation.
  • As a developer it would be a very common thing, if a binding/override-value has some kind of "slug". We know that from meta fields, block attributes, ... - the stored "slug"/"key" is often different from a label that we expose to the user.
  • And a slug - being a thing for code - should follow the naming in code. As a German-speaking developer, my variables, keys, classes are English - but labels and names are German - because the user has to understand them. Example: Renaming the block to "Überschrift" but the binding/override is named "heading".
  • As an editor-user I maybe still want to rename a block (if I've got the permissions), even though it's bound - because the pattern is from a theme and doesn't reflect my usage of the pattern and therefore naming might be misleading.

So thinking about implementation, for me that means:

  • Decouple renaming blocks and enabling overrides - I don't think they should even be located near each other.
  • Renaming blocks should have no side-effects. Just like renaming a layer in Figma & Photoshop.
  • When enabling overrides, the "key" could be generated by slugifying the current renamed block name - or if not renamed yet, an automatically generated ID.
  • Changing the "key" in pattern-code (code editor, file) should be possible.
  • Thinking about shuffling patterns, pattern overrides have two distinct things: "data schema" (which is the keys of the overrides) and "labeling" (block names, for display in the editor). That's how most of the things in core work (register_meta, register_settings, block attributes).

@getdave
Copy link
Contributor

getdave commented Apr 10, 2024

It could even tie into the fact that renaming in the list view also happens in a modal.

Just to note that renaming in the List View already uses a modal. We were unable to land the "inline" renaming approach because of a11y concerns.

My feeling is that it might be good if the user is more guided towards using a meaningful name for the blocks because of the desire for the names to have value as a schema.

I think Dan's instinct is good here. Indeed this is precisely the approach we took with the current block renaming. If you're renaming a block I think it's reasonable to ask for a custom name, especially if that's going to be connected to overrides.

UI/UX

My preferred UI is the button + modal pattern. Why? Because the action requires explicit opt in:

  • click button confirming you wish to enable the overrides feature.
  • give the block a specific name (within the known context of overrides)
  • toggle the fields to be affected

@talldan
Copy link
Contributor Author

talldan commented Apr 16, 2024

I'll close this issue, as a PR was merged that improves the flow - #60234.

There's also some further iterations underway in #60769, and an issue tracking that - #60760.

Thanks so much for everyone's input. ❤️

@talldan talldan closed this as completed Apr 16, 2024
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 Needs Design Needs design efforts. [Type] Enhancement A suggestion for improvement.
Projects
No open projects
Status: Done
Development

No branches or pull requests

8 participants