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

Navigation on Browse Mode: Add navigation list view to the template part editor #50704

Closed
scruffian opened this issue May 17, 2023 · 12 comments · Fixed by #51492
Closed

Navigation on Browse Mode: Add navigation list view to the template part editor #50704

scruffian opened this issue May 17, 2023 · 12 comments · Fixed by #51492
Assignees
Labels
[Block] Navigation Affects the Navigation Block [Block] Template Part Affects the Template Parts Block Needs Design Feedback Needs general design feedback.

Comments

@scruffian
Copy link
Contributor

As part of #50396 it would be good to add a Navigation List View to the template part when a Navigation block is part of a template part. This is just a very rough mockup. cc @jasmussen for a better one!

Screenshot 2023-05-17 at 15 58 25
@scruffian scruffian added [Block] Navigation Affects the Navigation Block [Block] Template Part Affects the Template Parts Block labels May 17, 2023
@jasmussen
Copy link
Contributor

jasmussen commented May 18, 2023

How's this?

Navigation in Template parts

CC: @jameskoster

@jasmussen jasmussen added the Needs Design Feedback Needs general design feedback. label May 18, 2023
@jameskoster
Copy link
Contributor

That looks like good for template parts containing a single menu.

  • How would it handle multiple menus?
  • Is "Header navigation" the menu name, or a generic label?
  • If the Navigation contains a site logo block, should we surface the control for editing it?

Appreciate it's not something to address here but we really need to fix the frame dimensions when displaying template parts 🙈

@jasmussen
Copy link
Contributor

How would it handle multiple menus?

Good one. Super edge-case, but good one. How about, one subheading per nav block?

Is "Header navigation" the menu name, or a generic label?

Name of the navigation. I believe it should already smartly name it something like that, if the navigation is created when it's inside a Header template part.

If the Navigation contains a site logo block, should we surface the control for editing it?

Yes, I omitted that to keep the design on point, but @scruffian if you'd like to see that, let me know!

Appreciate it's not something to address here but we really need to fix the frame dimensions when displaying template parts 🙈

Right — do you mean in trunk, where it's full height? Or in the mockup?

@jameskoster
Copy link
Contributor

I don't think it's super edge case. Maybe more of a footer thing... something like this would require multiple menus:

Screenshot 2023-05-18 at 13 21 02

Stacking menus might be a bit fragile, especially when there are more than two. Another option would be to handle multiple menus the same way we intend to handle them in the Navigation panel, which I assume would be drilldowns:

Header with multiple menus

Right — do you mean in trunk, where it's full height? Or in the mockup?

Sorry, on trunk – the frame is full-height regardless of the template part height. Your mockup is correct.

@jasmussen
Copy link
Contributor

Stacking menus might be a bit fragile, especially when there are more than two. Another option would be to handle multiple menus the same way we intend to handle them in the Navigation panel, which I assume would be drilldowns:

Drilldowns could work, but then we have the issue of editing just the generic style-book version of the navigation menu, without the context of the template part. That's why I'd default to them stacked first and foremost. But if the implementation is simpler with the drilldowns and stylebook nav editing, that's fine too.

Sorry, on trunk – the frame is full-height regardless of the template part height. Your mockup is correct.

👌

@jameskoster
Copy link
Contributor

Good point.

I think we'll end up needing a way to toggle visibility when there are multiple menus (accordions?) so you can focus, and avoid dragging items from one menu to another (would that even work?), but let's try stacking.

@getdave
Copy link
Contributor

getdave commented May 23, 2023

How about (when you have multiple menus) you list all the menus in the template (as shown) but then when you click one it takes you to the Navigation section and focuses on the individual menu section. This avoids us having drill downs for Menus within the drill down for the template part which I think we should avoid.

@scruffian
Copy link
Contributor Author

Another aspect to consider here is that users might want to switch the navigation that a template is using. I wonder if there's a way to surface that action in these designs as well?

@jameskoster
Copy link
Contributor

jameskoster commented May 23, 2023

How about (when you have multiple menus) you list all the menus in the template (as shown) but then when you click one it takes you to the Navigation section and focuses on the individual menu section.

Wouldn't you lose the situational context with this approach? The strongest argument for stacking is that you get to see the impact of your changes in the context of the template part.

You did inspire another option for multiple menus though: list each one and open edit mode with the associated Navigation block selected on click. It's not as elegant as stacking, but I do worry how stacking will scale.

Screenshot 2023-05-23 at 10 51 36

Another aspect to consider here is that users might want to switch the navigation that a template is using. I wonder if there's a way to surface that action in these designs as well?

Possibly, but probably worth looking at separately.

@getdave
Copy link
Contributor

getdave commented Jun 14, 2023

How about

  • we land this with just showing the 1st menu that's in the template (as per original design)
  • we follow up with change that will list all menus (if there's more than one) and allow click through to Nav focus mode for editing of each

That avoids the stacking, which I agree won't scale and I bet will introduce a11y problems.

@jameskoster
Copy link
Contributor

I'd be a little anxious about assuming the first menu in the template to be the primary one. For instance a store might have a small menu right at the top of the page with links such as 'log in/out', 'my account', 'cart', and nothing else – the experience would be a little strange if that was prioritised over the main menu.

But if the suggested follow-up landed before 6.3 then it's probably fine.

@getdave
Copy link
Contributor

getdave commented Jun 14, 2023

Actually I've decided I'll just land it in a single PR. Watch this space.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Block] Template Part Affects the Template Parts Block Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants