Components: Assess stabilization of Navigator

[fullofcaffeine]

Issue: #59418.

What?

This is part of a larger effort to remove __experimental prefix from all "experimental" components, effectively promoting them to regular stable components. See the related issue for more context.

Why?

The strategy of prefixing exports with __experimental has become deprecated after the introduction of private APIs.

How?

1. Export it from components without the __experimental prefix;
2. Keep the old __experimental export for backwards compatibility;
3. Change all imports of the old __experimental in GB and components to the one without the prefix (including in storybook stories). Also, update the docs to refer to the new unprefixed component;
4. Add the component storybook id (get it from the storybook URL) to the PREVIOUSLY_EXPERIMENTAL_COMPONENTS const array in manager-head.html so that old experimental story paths are redirected to the new one;
5. Add a changelog for the change.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @lysyjan.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: lysyjan.

Co-authored-by: fullofcaffeine <[email protected]>
Co-authored-by: youknowriad <[email protected]>
Co-authored-by: jsnajdr <[email protected]>
Co-authored-by: jeryj <[email protected]>
Co-authored-by: ciampo <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[youknowriad]

To be honest, I'm hesitant here. We moved away from this component recently in the site editor. It's still used in a few places but could largely be simplified as it's main purpose seem to be Animation and Focus Management. I can be convinced otherwise though :)

What do you think @jsnajdr

[jsnajdr]

First of all the Navigator components are already quite widely used by plugins, even with the experimental prefix.

WooCommerce uses it to implement a navigation sidebar in Customize Store. It's the same code as Site Editor before #60466, including the useSyncPathWithURL hook, there's a lot of copy&paste. Seems we'll need to do the same refactoring in Woo as we did in Core.

Then there's MailPoet, which uses Navigator to implement a styles sidebar. The code is still a stub, with TODO labels. @lysyjan might know what they plan to do with it. This usage of Navigator looks legit to me -- similarly to what Core does in the Global Styles sidebar, it's used to implement a complex sidebar with nested screens, with no connection to page history and routing.

Then I found the Otter Blocks plugin, which uses Navigator to implement its settings sidebar:

This again is a legitimate use, Core uses it the same way.

Another plugin is Visual Portfolio. I didn't install this one, but from the code it looks like an another example of a legitimate plugin settings sidebar.

Therefore, it seems like a good idea to finally stabilize Navigator. Before we do that, we should review the API and consider if we want to change something. My candidates would be removal of the goToParent method (I don't see it used), and always forcing backPath links on sidebar back buttons.

[lysyjan]

Then there's MailPoet, which uses Navigator to implement a styles sidebar. The code is still a stub, with TODO labels. @lysyjan might know what they plan to do with it.

@jsnajdr The current implementation of the styles sidebar in the new email editor is behind a feature flag. We are working on adding the next NavigatorScreens, and we follow implementation in the Global Styles sidebar, as the design is very similar.

[jeryj]

We're using this Navigator for pattern category navigation. In there, I used the <Composite /> to make the navigator one tab stop and use arrow keys to move between items. I'd love to see this be a part of the component so semantic menu markup and interactions can be used out of the box with the Navigator. This could include Escape keypresses to move back a level.

[ciampo]

removal of the goToParent method (I don't see it used)

Just pinging @youknowriad to make sure he's ok with removing the goToParent method, as he proposed the initial addition — I'm also a bit hesitant in removing code that was merged in a previous WordPress release, but the fact that you couldn't find any usages should be reassuring.

Also, goToParent allowed hierarchical navigation and it also allowed Navigator to navigate "back" (up to parent) even when no location history was provided (ie. form the initial screen).

and always forcing backPath links on sidebar back buttons.

@jsnajdr I'm not sure this is feasible, given that Navigator could be potentially controlled by UI external to NavigatorScreens and even just programmatically following other user interaction?

[...] I used the <Composite /> to make the navigator one tab stop and use arrow keys to move between items. I'd love to see this be a part of the component

@jeryj I don't think this is something that Navigator should implement, as explained in this comment:

I think that Navigator (not to be confused with the deprecated Navigation 😅 ) should not implement Composite internally. Navigator's goal is to allow navigation across different views. The component is completely agnostic about what such views render, and therefore it can't assume what keyboard behavior should be implemented. The only focus management-related assumption made by the component is that focus should be moved to the new view after each transition.

[youknowriad]

I don't mind the removal of goToParent but from my perspective goBack is more problematic (unexpected behavior in most cases) than goToParent.

[ciampo]

I'm personally also OK keeping goToParent around — @jsnajdr do you feel strongly about dropping it?

[jsnajdr]

I've been looking at goToParent removal recently and all we can do is to mark it as deprecated. Because it's used by WooCommerce. The latest bleeding edge version no longer uses it, but there are many older versions in the wild.

[jsnajdr]

I'm not sure this is feasible, given that Navigator could be potentially controlled by UI external to NavigatorScreens and even just programmatically following other user interaction?

To control the Navigator programatically, there is the useNavigator().goTo( path ) method. Do we really need anything else?

goBack is for going back in history, and I'm not sure if Navigator really needs history at all. Other UI components, for example tab panels, also don't keep history of the tabs that you visited.

goToParent depends on the assumption that the parent of /a/b/c is always /a/b. And not /a or something else. That is not true 100% of the time.

[youknowriad]

goToParent depends on the assumption that the parent of /a/b/c is always /a/b. And not /a or something else. That is not true 100% of the time.

I agree but it's better than history which is very random, specially if there's programatic navigation in the middle. When you see the back button in the Navigation UI, you expect it to go up the tree of navigation, not back in history. In that sense I prefer goToParent behavior over goBack for a UI component. As opposed to "TabPanels", "Navigator" is a tree of screens, so going up and down makes sense. I don't see any valid use-case for Navigator where it's not really a tree.

[ciampo]

It's definitely important that we agree on the APIs that Navigator should expose before stabilising it.

What should the role of Navigator be? And, in particular, how much "routing" logic should the component include?

A brief recap

From what I recall, when it was created, the main idea behind Navigator was to:

• offer an easy way to swap different "views" within the same screen space
• animate the swap and offer two different animations (go to new location vs going back to previous location)
• do not provide any router-like functionality, and do not sync with the document's location
• provide any necessary accessibility feature (ie focus management when swapping view)

For its first implementation, we opted for storing an array of location history. Navigating to a new location would push a new location item to the stack, and going back would pop an item from the stack. This was a simple implementation, but it allowed "back" links to be agnostic of the previous history locations, and it allowed to store a reference to which element focus should be moved to when navigating back.

Later on, as the site editor started to become more complex, we needed a way to go "back" to what, in the hierarchy of screens was the "previous" screen, even if the user didn't actually visit that screen. The "location stack" technique couldn't help here, and that's why we introduced the concept of "goToParent".

Moving forward

One potential approach could be to greatly simplify Navigator, removing any history/routing logic.

In practical terms:

• we could remove (deprecate) goToParent, goBack, <NavigatorBackButton />, <NavigatorToParentButton />;
• the component wouldn't store an internal location stack (history) anymore
• consumers of Navigator would only use goTo() and <NavigatorButton /> to navigate across views, using the isBack prop to trigger a "backwards" animation, and they would always need to specify the path of the new view.

Navigator would basically delegate any routing logic to its consumers.

What do you think?

[jsnajdr]

Navigator would basically delegate any routing logic to its consumers.

I'm not sure about this, because as @youknowriad says, navigator is a tree:

As opposed to "TabPanels", "Navigator" is a tree of screens, so going up and down makes sense.

And the back button is practically always there so it makes sense to somehow support it natively:

The problem is that a generic "go back" solution that works for everyone is possible only for simple trees where each node has one parent:

Here it doesn't even matter whether we use goBack with history, or goToParent with removing the last / path element, because they are exactly equivalent. There is only one way how you could arrive at a certain node: from the parent. So both implementations give the same result.

But in Site Editor we eventually ended up doing cross-tree navigations where there were multiple ways to get to, e.g., a pattern page:

Also, the pattern matching and named arguments from #47827 make the tree more complicated.

So there is a tradeoff: as the tree gets more complicated, the generic goBack/goToParent methods start to be less useful, as they don't reliably work for everyone.

This leads me to a question: how complex trees do we want to support in Navigator? Now when we stopped using Navigator for top-level SPA routing, we could simplify it again.

[youknowriad]

This leads me to a question: how complex trees do we want to support in Navigator? Now when we stopped using Navigator for top-level SPA routing, we could simplify it again.

Yes, this my take as well. The component has always been about rendering a simple "tree" of screens where there's only one way to go up and down.

I think we still want to support a way to go to a particular place in tree programmatically, but clicking the back button should always navigate up the tree for me.

[ciampo]

One way to simplify Navigator then could be to:

• remove any location history-related logic;
• deprecate goToParent and NavigatorToParentButton
• have goBack and NavigatorBackButton basically behave like goToParent

The only thing is that "back" navigation will break for consumers of Navigator that don't use hierarchic paths, so in case we'd need to provide clear warnings / dev notes.

Also, without storying location history, we'd likely lose the ability to restore focus to the element that generated the view transition (we can still focus the first focusable element on the view)

[jsnajdr]

without storying location history, we'd likely lose the ability to restore focus to the element that generate the view transition

Well that's a reason to keep the location history 🙂 The point is that goBack and goToParent do the same thing if the structure is a strict tree, so one of them is superfluous. And we can choose which one to remove.

"back" navigation will break for consumers of Navigator that don't use hierarchic paths

For public plugins, we can review all usages (in plugins like Otter Blocks or Visual Portfolio), there are not so many. And I think all of them are going to use hierarchical paths. I don't remember seeing anything unusual when I was reviewing them 2 months ago.

[ciampo]

Well that's a reason to keep the location history 🙂 The point is that goBack and goToParent do the same thing if the structure is a strict tree, so one of them is superfluous.

I was just a bit confused by the few mentions about "largely simplifying" the component. If we keep the location history AND the "go back to parent" functionality, then most of the implementation won't be touched.

And we can choose which one to remove.

I personally think that keeping goBack when the functionality is actually "go to the parent screen" may be confusing to the consumers of the component. goToParent is more verbose but it is IMO more clear.

[jsnajdr]

If we keep the location history AND the "go back to parent" functionality

What I'm trying to say is that for most usages, "go back in location history" and "go to parent path" are identical. We can remove one of them and no one will notice. It's only when your navigation structure is no longer a tree, but a more complicated graph, when it starts to make a difference because then the "back" location is no longer unique.

I personally think that keeping goBack when the functionality is actually "go to the parent screen" may be confusing to the consumers of the component.

I think the meaning is very clear: it's what the "back" button in the UI does:

Using the goToParent or goBack method you can implement the back button, or do the equivalent navigation programatically.

[ciampo]

I was reflecting on how currently Navigator restores focus when navigating "back" — restoring focus on the element that originally generated the "forward" navigation is great, but I think we could treat it as a "nice to have". I'd argue that the most important thing, focus management-wise, is that Navigator moves focus to the new screen (whether on the first focusable element, or on the one that generated the forward transition, that matters less).

I also don't think that browsers restore focus this way when navigating back, nor the most used third-party routers.

With this in mind, I'm thinking that it could be worth dropping the location history logic completely and fallback to a simpler focus restoration strategy, which would allow us to simplify Navigator quite a lot. What do you think?

[jeryj]

With this in mind, I'm thinking that it could be worth dropping the location history logic completely and fallback to a simpler focus restoration strategy, which would allow us to simplify Navigator quite a lot. What do you think?

Having it restore focus to the previously activated navigation item is very nice for keyboard navigation. It makes things much simpler and faster to use. Losing this behavior would be a big step backwards, IMO.

[ciampo]

Got it.

Judging by the conversations above, it looks like the only simplification to Navigator that we'll be able to apply before stabilizing is to deprecate either goBack or goToParent, and have them behave the same (go to parent navigation).

Location history stack should be kept around for focus restoration purposes.

I will start working on this soon and post updates.

[jsnajdr]

I agree that the focus restoration is a very nice feature. Few apps do this today and the code is ugly, but there is a big benefit for the keyboard user.

macOS Finder restores the last selected item when going up in the filesystem hierarchy (⌘↑ and ⌘↓):

finder-keyboard-nav.mov

Interesting that the web version of Dropbox also has a very similar file navigator UI, but keyboard navigation is broken: when going up, you lose the selection.

[ciampo]

Closing in favour of #64613