-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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 Component: Split Stories to Improve Their Consumption #25572
Conversation
Size Change: +1.91 kB (0%) Total Size: 1.17 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, this is a much needed change. Thank you @Copons, this will make adoption much easier.
Default: the Navigation component at its minimum. Folks can start here to understand how it can be composed.
Since we removed internal activeItem
logic, even the default will need activeItem
logic.
const [ activeItem, setActiveItem ] = useState( 'item-1' );
You can see in the storybook it doesn't work very well.
I think the distinction in stories lies more in the various ways NavigationItem
can be used. I do like the default being the bare minimum to making something useful. What it if had the minimum state
and a few NavigationMenu
and NavigationItem
with no custom content, only titles and hrefs?
Then subsequent stories can highlight groups, custom content, and various ways of setting the active item from outside or mocked router links?
369e653
to
2edb1d4
Compare
@psealock Thanks for the suggestions! I've done almost all you said in 369e653. As you might notice, items have no I've also noticed a bug with "half-controlled" menus caused by a (With "half-controlled" I mean "active item controlled externally, active menu controlled by the Nav itself".) EDIT: issue should be sorted out by #25608. EDIT 2: fixed |
I'm merging this now, but of course feel free to update it as you see fit, in case you notice some odd behaviour or think of a clearer way to showcase the component! ✨ |
Description
While working on the Navigation component, I've realized that every time we add a new feature, we just update the one story with, for example, a new navigation item using the new feature.
IMHO this has quickly become unreadable, the story a mix of all kinds of stuff (its navigation is sometimes controlled and sometimes uncontrolled, there are examples hidden in sub-menus, etc.)
In this PR I've split it into 3 separate stories:
Navigation
component at its minimum. Folks can start here to understand how it can be composed.When we introduce a new feature (e.g. the Search), we should consider if it's concise enough that can belong to an existing story, or if we should create a new one instead.
How has this been tested?
npm run storybook:dev
Types of changes
Component Storybook update.
Checklist: