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

Make the navigation wrapping ul component user customisable #9153

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mikehazell
Copy link
Contributor

@mikehazell mikehazell commented May 22, 2024

The NavigationContainer component rendered it's children inside a ul meaning if you wanted to render anything other than an li you would have to do some gymnastics to make it work.

This change makes it the users' responsibility to properly wrap list items in a ul.

  • exposes a NavItemGroup component with appropriate styling
  • updates "Custom Admin UI Navigation" usage docs

@@ -0,0 +1,8 @@
---
"@keystone-6/core": major
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically a breaking change? ... custom Navigation in the Admin UI will render weirdly or have invalid DOM structure if not updated.

Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 20fe773:

Sandbox Source
@keystone-6/sandbox Configuration

Comment on lines +319 to +352
### ListNavItem

This component will render a single `NavItem` for the given list.

```tsx
import { ListNavItem } from '@keystone-6/core/admin-ui/components'
```

In this example we create groups for our lists.

```tsx
const listGroups = [
[
{ name: 'People', lists: ['User', 'Bio', 'Role']},
{ name: 'Posts', lists: ['Post', 'Category', 'Tag']},
]
];

const CustomNavigation = ({ lists }) => (
<NavigationContainer>
{listGroups.map(group => (
<Box key={group.name}>
<H5 paddingX="xlarge">{group.name}</H5>
<NavItemGroup>
{group.lists.map((key) => {
const list = lists.find((l) => l.key === key);
return list ? <ListNavItem key={key} list={list} /> : null;
})}
</NavItemGroup>
</Box>
))}
</NavigationContainer>
)
```
Copy link
Contributor Author

@mikehazell mikehazell May 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind of a contrived example. You could do something like this1 with the ListNavItems component too but I wanted to show that this component is also exported.

Footnotes

  1. Kind of like this. Using <ListNavItems includes={[...]} you don't have control over the order in which the lists are shown, only which lists.

@mikehazell mikehazell marked this pull request as ready for review May 22, 2024 04:43
```

{% hint kind="warn" %}
Versions of `@keystone-6/core` before `1.2.0` wrapped all children of `NavigationContainer` with the `NavItemGroup` component.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've guessed what the version number will be. Also I'm not sure we need to call out that this thing has changed.

@dcousens
Copy link
Member

Likely superseded by #9253

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants