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

Components: Update Navigator components — Tracking Issue #34907

Open
8 of 15 tasks
ciampo opened this issue Sep 17, 2021 · 4 comments
Open
8 of 15 tasks

Components: Update Navigator components — Tracking Issue #34907

ciampo opened this issue Sep 17, 2021 · 4 comments
Labels
[Feature] Component System WordPress component system [Package] Components /packages/components [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues.

Comments

@ciampo
Copy link
Contributor

ciampo commented Sep 17, 2021

This is an overview issue to track all of the tasks necessary to introduce and refine the set of Navigator components, especially in the context of using it in the updated Global Styles Sidebar.

As discussed in numerous previous issues and PRs (#34885, #32923, #34904), the current set of Navigation components feels limited, as it is quite tailored to representing nested menus, and has a quite imperative style.

For these reasons, we would like to replace the current set of Navigation components with a new set of components (for now called Navigator), focused on offering a simple set of APIs to navigate through different "screens" by simply specifying the current screen's path in a simple, declarative way. This new set of components should not be rendering any opinionated pieces of UI.

Improve the component

Adoption strategy

This is a potential strategy, although it needs to be validated:

  • Rewrite the legacy Navigation components to use the new Navigator components under the hood
  • Replace all usages of the legacy Navigation with the new Navigator components (including all known external projects, like Woo)
  • Maybe mark the legacy Navigation components as deprecated?
  • Maybe move the legacy Navigation components away from the components package, into the app code?
@ciampo ciampo added [Package] Components /packages/components [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues. [Feature] Component System WordPress component system labels Sep 17, 2021
@ciampo
Copy link
Contributor Author

ciampo commented Sep 17, 2021

A brief recap of what's happened so far:

  • In Global Styles v2: Move prototype from G2 #32923 we started to try and move the Global Styles Sidebar prototype from Global Styles Sidebar (Design/Code V2) ItsJonQ/g2#293 into Gutenberg as a Storybook example. We agreed to keep the Navigation/Routing set of components coming from the prototype, as they seemed to work well enough. The plan was to focus on other areas of the prototype, and come back to the navigation component later
  • In Update the global styles sidebar to use a navigation component #34885 we adopted a different approach — we started working on the Global Styles Sidebar by editing the current Sidebar directly. After discussing which set of Navigation components should be used for this new version of the Sidebar, we decided to start with the "legacy" set already available in @wordpress/components in order to unblock the work on other areas of the project. We also agreed that, in parallel, we should on a new set of Navigation components (getting inspired by the components from the prototype mentioned above)

One of the most recent points of conversation has been about having the Navigation components touching the browser's URL. In https://github.com/WordPress/gutenberg/pull/32923/files#r710975447 @youknowriad has argued that

our components in @wordpress/components shouldn't be exposing anything related to URLs at all as this doesn't make sense in a react native context. If we want to sync the navigation state to URLs, we can still do it in the application layer (edit-site) like done here but I don't see it as a requirement for a start.

I personally believe that we should aim at having a complete set of Routing capabilities powering the Navigation components. The prototype from ItsJonQ/g2#293 features a Routing system heavily inspired by react-router. An idea could be to:

  • potentially use react-router instead of the custom Routing components (which should also support native environments)
  • Have the Navigation components use react-router under the hood. This set components should offer a way to wrap different "screens" and to link to a screen (both via React compoments and via hooks). They should assume a dom structure (e.g. be part of a menu) or a design language

With a feature-full router under the hood, we would be able to pick and choose which type of "routing" (e.g. in-memory, URL-based...) we want to expose from @wordpress/compoments.

@griffbrad
Copy link

griffbrad commented Sep 17, 2021

Regarding routing, Q's original feedback on the current Navigation component is worth revisiting:
#24107 (comment)

In particular, he describes how the requirements for the component align very well with the requirements for a router. The challenge with using React Router directly is that its state management and shared context may block using multiple instances of the router for different sub-sections of UI. @ockham and @sirreal chimed into urge further exploration of directly using React Router, though:
ItsJonQ/g2#20 (comment)

I agree that if we're integrating a routing API, we ought to explore using React Router directly further.

Beyond the routing concerns, though, I think the biggest issue with the current components is that they're designed particularly for navigation menus whereas the structure proposed in G2's Navigator component does not introduce opinionated styles for that use case. Beyond keeping in mind the other navigation-related requirements (e.g. nested layers, tracking "active" state, affecting things outside the menu, outside changes affecting the menu itself), sticking to a Reakit-like approach to styles seems important here so that these components aren't useful only for the particular menus we have in mind now but can be used in any part of the Gutenberg UI without introducing style conflicts.

This issue from G2 goes over some of these style-related requirements:

Note: Navigator and it's sub-components do not render styles (e.g. a dark background). This is by design. These components are effectively (smart) layout components. They are designed to focus on functionality. Aesthetic rendering should be provided by enhancing the sub-components and from the content itself.

@stokesman
Copy link
Contributor

stokesman commented Oct 4, 2021

Fix a bug where a horizontal scrollbar would appear during a transition

I've made #35310 as an example of fixing that without worrying about it within the component for now. Don't let the number of changes throw you off. The last two commits are extraneous to the scrollbar fix. They're outdated CSS cleanup and alignment improvement for the Post Editor’s preferences modal. I may just break them off for a separate PR.

In addition to that, I wanted to suggest we improve the animation in the component by having the exiting screen also animate. Apparently, it was that way in the g2 repo and we've got some dregs of that still leftover in ours. I've got #35312 open that restores the framer-motion exit animations. Also, because I was curious about performance differences there's #35311 which does the same but with CSS driven animation.

@ciampo
Copy link
Contributor Author

ciampo commented Oct 4, 2021

Thank you @stokesman for taking the time to contribute to the Navigator component! I've left individual comments on each of the issues that you linked.

Navigator is a very recent component and should be still considered highly experimental. We are purposefully introducing it in its simplest form and adding features or fixes as we experiment with its usage across a few places in Gutenberg. This means that, at the moment, development on the component may move very quickly and change direction often.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Component System WordPress component system [Package] Components /packages/components [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues.
Projects
None yet
Development

No branches or pull requests

3 participants