feat(ui): add onToggle callback to SideNavigationGroup and SideNavigationItem#1803
Open
edda wants to merge 4 commits into
Open
feat(ui): add onToggle callback to SideNavigationGroup and SideNavigationItem#1803edda wants to merge 4 commits into
edda wants to merge 4 commits into
Conversation
…tionItem Adds an `onToggle(isOpen)` callback to both components that fires whenever the user expands or collapses the section. Lets parents observe (and optionally drive) the open state without changing the existing `open` prop behavior. Closes #1799. Signed-off-by: Esther Schmitz <esther.schmitz@sap.com>
🦋 Changeset detectedLatest commit: 4694f3f The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an onToggle(isOpen: boolean) callback to SideNavigationGroup and SideNavigationItem to let consumers observe (and optionally drive) expand/collapse behavior while preserving the existing “internal state seeded by open and re-synced on prop changes” semantics.
Changes:
- Added
onToggle?: (isOpen: boolean) => voidtoSideNavigationGroupandSideNavigationItemand call it with the next open state on user toggles. - Added Vitest coverage for
onToggle, disabled behavior, andopenre-sync behavior. - Added Storybook “Controlled” examples demonstrating parent-owned open state.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ui-components/src/components/SideNavigationItem/SideNavigationItem.component.tsx | Adds onToggle prop and triggers it from the chevron toggle handler; updates prop typings. |
| packages/ui-components/src/components/SideNavigationGroup/SideNavigationGroup.component.tsx | Adds onToggle prop and triggers it from the group row click handler; updates prop typings. |
| packages/ui-components/src/components/SideNavigationItem/SideNavigationItem.test.tsx | Adds tests for item onToggle, label-vs-chevron behavior, disabled behavior, and open re-sync. |
| packages/ui-components/src/components/SideNavigationGroup/SideNavigationGroup.test.tsx | Adds tests for group onToggle, internal toggling without callback, and open re-sync. |
| packages/ui-components/src/components/SideNavigationItem/SideNavigationItem.stories.tsx | Adds “Controlled” story demonstrating parent-managed open via onToggle. |
| packages/ui-components/src/components/SideNavigationGroup/SideNavigationGroup.stories.tsx | Adds “Controlled” story demonstrating parent-managed open via onToggle. |
| .changeset/sidenavigation-on-toggle.md | Adds a minor changeset documenting the new callback behavior. |
Clarify the `open` JSDoc on both components so it reflects the seed-and-resync semantics rather than implying strict React control, and query the label button in the SideNavigationItem onToggle test by accessible name instead of position to make the test robust to DOM changes. Signed-off-by: Esther Schmitz <esther.schmitz@sap.com>
… stories Storybook v8/v9 dropped the implicit `on*` action inference, so handlers were no longer being logged in the Actions panel for these stories. Add explicit `fn()` spies as default args for `onToggle` (and `onClick` on the Item story) so the Actions panel reflects user interactions, while the Controlled stories keep their real state-based handlers. Signed-off-by: Esther Schmitz <esther.schmitz@sap.com>
…n items
Wrap nested onToggle handlers with `action("onToggle")` so the Actions
panel logs toggle events for nested items/groups. The Storybook `fn()`
spy only auto-routes to the panel when bound via `args`; inline JSX
props need the explicit `action()` helper.
Signed-off-by: Esther Schmitz <esther.schmitz@sap.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
onToggle?: (isOpen: boolean) => voidtoSideNavigationGroupandSideNavigationItem. The callback fires whenever the user clicks the toggle affordance, with the next open state.SideNavigationGroupthe whole row is the toggle, soonTogglefires on any click of the group. ForSideNavigationItemonly the chevron is the toggle — the label keeps its existingonClick/hrefnavigation behavior unchanged.openprop semantics are preserved: it still seeds the initial state and re-syncs the internal state when the parent re-renders with a new value. No breaking changes — consumers that don't passonTogglesee identical behavior to today.This is the same soft-control pattern already used by
PanelandModalin this library and lets parents implement use cases like "auto-open the group containing the active route while preserving any groups the user has manually closed" (the Aurora Dashboard use case from the issue).Closes #1799.
Test plan
pnpm vitest runinpackages/ui-componentsforSideNavigation*suites — 47/47 passing, including 13 new tests:onTogglefires withtrue/false, internal state still toggles without callback,openprop re-sync still works.onTogglefires from the chevron with the next state; label clicks do not fireonToggle(onlyonClick); disabled items don't fireonToggle; internal state still toggles without callback;openprop re-sync still works.pnpm --filter @cloudoperators/juno-ui-components typecheckclean.pnpm --filter @cloudoperators/juno-ui-components lintclean.Controlledexample for both components demonstrating the parent-owned-state pattern. Verify visually in Storybook.Controlledstories in Storybook behave as expected.Notes
SideNavigationItemPropsnow usesOmit<HTMLAttributes<HTMLElement>, "onToggle">to avoid colliding with the DOMToggleEventhandler type onHTMLAttributes. OuronToggle(isOpen: boolean)is a domain-level callback, not a DOM event handler.minor).