Open
Conversation
🦋 Changeset detectedLatest commit: 01270a9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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
There was a problem hiding this comment.
Pull request overview
Introduces a new KdsNavigation layout component (with KdsNavItem) to render a vertical navigation menu with selection tracking via v-model, and refactors the former list-item divider into a shared Divider accessory.
Changes:
- Add
KdsNavigation+KdsNavItemcomponents, types, and Storybook stories. - Introduce
KdsNavigationItem/KdsNavigationPropstyping for navigation item lists and selection. - Replace the former list-specific divider usage with a shared
Dividercomponent and update list container styling accordingly.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/components/src/layouts/Navigation/types.ts | Adds public types for navigation props/items. |
| packages/components/src/layouts/Navigation/KdsNavItem/types.ts | Defines KdsNavItem props/types (incl. accessories). |
| packages/components/src/layouts/Navigation/KdsNavItem/KdsNavItem.vue | Implements the nav item rendering and interaction. |
| packages/components/src/layouts/Navigation/KdsNavItem/KdsNavItem.stories.ts | Adds Storybook coverage and play tests for nav item. |
| packages/components/src/layouts/Navigation/KdsNavItem/index.ts | Exports KdsNavItem and its types. |
| packages/components/src/layouts/Navigation/KdsNavItem/enums.ts | Adds nav item enum constants. |
| packages/components/src/layouts/Navigation/KdsNavigation.vue | Implements navigation list + v-model selection. |
| packages/components/src/layouts/Navigation/KdsNavigation.stories.ts | Adds Storybook coverage and play tests for navigation. |
| packages/components/src/layouts/Navigation/index.ts | Exports navigation module entry point. |
| packages/components/src/layouts/index.ts | Re-exports Navigation from layouts barrel. |
| packages/components/src/forms/_helper/List/ListItemDivider/index.ts | Removes the old divider export (legacy helper). |
| packages/components/src/forms/_helper/List/ListContainer/KdsListContainer.vue | Switches list separators to use shared Divider and adds local divider class styling. |
| packages/components/src/accessories/Divider/index.ts | Adds barrel export for Divider (currently incorrect export form). |
| packages/components/src/accessories/Divider/Divider.vue | Renames divider root class from list-specific to generic. |
| packages/components/src/accessories/Divider/Divider.stories.ts | Updates Divider story to match new component name/location. |
| .changeset/shaggy-kids-carry.md | Adds a changeset for publishing the new navigation component. |
Comments suppressed due to low confidence (1)
packages/components/src/accessories/Divider/Divider.stories.ts:17
- Story titles under
packages/components/src/accessories/*consistently use theAccessories/...prefix (e.g.Accessories/Avatar,Accessories/Badge). This story usestitle: "Divider", which makes it inconsistent and harder to find in Storybook’s hierarchy; consider usingAccessories/Divider(or another consistent category).
thresharx
force-pushed
the
KDS-573-navigation
branch
from
50521a9 to
eb1a00f
Compare
thresharx
force-pushed
the
KDS-573-navigation
branch
from
eb1a00f to
09f738d
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces a new KdsNavigation layout component (with KdsNavItem) for rendering vertical navigation menus, and refactors the existing list divider helper into a reusable Divider accessory.
Changes:
- Add
KdsNavigation+KdsNavItemcomponents with Storybook coverage and public exports underlayouts/. - Refactor list item divider usage to the
accessories/Dividercomponent and updateKdsListContaineraccordingly. - Add a changeset for releasing the new navigation component.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/components/src/layouts/index.ts | Re-export new Navigation layout module. |
| packages/components/src/layouts/Navigation/types.ts | Defines KdsNavigationProps and item types. |
| packages/components/src/layouts/Navigation/index.ts | Exports KdsNavigation and KdsNavItem. |
| packages/components/src/layouts/Navigation/KdsNavigation.vue | Implements navigation rendering + v-model selection. |
| packages/components/src/layouts/Navigation/KdsNavigation.stories.ts | Adds stories + play tests for navigation. |
| packages/components/src/layouts/Navigation/KdsNavItem/types.ts | Defines nav item props (label/link/icons/badge/button/etc.). |
| packages/components/src/layouts/Navigation/KdsNavItem/index.ts | Exports KdsNavItem + types. |
| packages/components/src/layouts/Navigation/KdsNavItem/KdsNavItem.vue | Implements nav item button/link rendering + styling. |
| packages/components/src/layouts/Navigation/KdsNavItem/KdsNavItem.stories.ts | Adds stories + play tests for nav item. |
| packages/components/src/forms/_helper/List/ListItemDivider/index.ts | Removes export (file becomes empty). |
| packages/components/src/forms/_helper/List/ListContainer/KdsListContainer.vue | Switches separator rendering to Divider and adds local divider class. |
| packages/components/src/accessories/Divider/index.ts | Adds Divider barrel export. |
| packages/components/src/accessories/Divider/Divider.vue | Renames root class to .divider and removes list-specific sizing styles. |
| packages/components/src/accessories/Divider/Divider.stories.ts | Renames story/component references to Divider and adjusts story title/imports. |
| .changeset/shaggy-kids-carry.md | Adds a minor changeset for @knime/kds-components (“add KdsNavigation”). |
Comments suppressed due to low confidence (2)
packages/components/src/accessories/Divider/Divider.stories.ts:16
- Story title is inconsistent with the established naming used by other accessories (e.g.
Accessories/Badge). Consider usingAccessories/Dividerto keep the Storybook sidebar structure consistent.
packages/components/src/forms/_helper/List/ListItemDivider/index.ts:1 - This file is now empty. If
ListItemDividerhas been replaced byDivider, it’s clearer to delete this index file entirely (and/or the folder if unused) to avoid dead exports and confusion.
thresharx
force-pushed
the
KDS-573-navigation
branch
from
09f738d to
7177e4d
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new vertical navigation layout component to the KNIME Design System components package and introduces a reusable divider accessory, including Storybook documentation and export wiring.
Changes:
- Add
KdsNavigationandKdsNavItem(component + types + stories) and export them fromlayouts/. - Introduce an
accessories/Dividercomponent and refactorKdsListContainerto use it. - Add a changeset for publishing
@knime/kds-components.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/components/src/layouts/index.ts | Re-export Navigation from the layouts barrel. |
| packages/components/src/layouts/Navigation/types.ts | Add public prop/item types for KdsNavigation. |
| packages/components/src/layouts/Navigation/index.ts | Export KdsNavigation, KdsNavItem, and Navigation types. |
| packages/components/src/layouts/Navigation/KdsNavigation.vue | Implement KdsNavigation with v-model selection and optional separators. |
| packages/components/src/layouts/Navigation/KdsNavigation.stories.ts | Add Storybook docs/tests and visual coverage stories for KdsNavigation. |
| packages/components/src/layouts/Navigation/KdsNavItem/types.ts | Define KdsNavItem props including optional link/button/badge. |
| packages/components/src/layouts/Navigation/KdsNavItem/index.ts | Export KdsNavItem and its types. |
| packages/components/src/layouts/Navigation/KdsNavItem/KdsNavItem.vue | Implement the interactive nav item (button/link), selected/disabled styling, and truncation title. |
| packages/components/src/layouts/Navigation/KdsNavItem/KdsNavItem.stories.ts | Add Storybook docs/tests and visual coverage stories for KdsNavItem. |
| packages/components/src/forms/_helper/List/ListItemDivider/index.ts | Remove the old ListItemDivider export. |
| packages/components/src/forms/_helper/List/ListContainer/KdsListContainer.vue | Replace ListItemDivider usage with the new Divider component and adjust styling. |
| packages/components/src/accessories/index.ts | Export Divider from the accessories barrel. |
| packages/components/src/accessories/Divider/index.ts | Add barrel export for the new Divider component. |
| packages/components/src/accessories/Divider/Divider.vue | Introduce the Divider component markup/styling. |
| packages/components/src/accessories/Divider/Divider.stories.ts | Add Storybook docs/coverage stories for Divider. |
| .changeset/shaggy-kids-carry.md | Changeset to publish the new Navigation component. |
Comments suppressed due to low confidence (2)
packages/components/src/accessories/Divider/Divider.stories.ts:16
- Storybook titles for accessory components are consistently grouped under
Accessories/...(e.g.Accessories/Badge,Accessories/LoadingSpinner), but this story is registered as justDivider. Please change the title toAccessories/Divider(and if you decide to keep this component internal-only, consider moving it into an internal Storybook section instead of exporting it publicly).
packages/components/src/accessories/Divider/Divider.vue:7 - The Divider component uses an unprefixed CSS class name (
divider). In this repo, component-scoped classes are consistently prefixed withkds-(e.g..kds-badge,.kds-avatar). Please rename the class to something likekds-divider(and update the selector accordingly) to match the established convention.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new Navigation layout component (with individual NavItem building block) and introduces a shared Divider accessory, replacing the previous list-specific divider usage.
Changes:
- Export
KdsNavigation(and related types) from the layouts package. - Add
KdsNavigation+KdsNavItemcomponents with Storybook coverage. - Introduce
KdsDivideraccessory and migrateKdsListContaineraway from the removedListItemDivider.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/components/src/layouts/index.ts | Re-exports the new Navigation module from layouts. |
| packages/components/src/layouts/Navigation/types.ts | Defines public prop/types for navigation items and the navigation component. |
| packages/components/src/layouts/Navigation/index.ts | Exposes KdsNavigation (and KdsNavItem) + types via folder index. |
| packages/components/src/layouts/Navigation/KdsNavigation.vue | Implements the navigation list component with v-model selection. |
| packages/components/src/layouts/Navigation/KdsNavigation.stories.ts | Storybook stories + interaction test for navigation selection. |
| packages/components/src/layouts/Navigation/KdsNavItem/types.ts | Defines props for the nav item building block. |
| packages/components/src/layouts/Navigation/KdsNavItem/index.ts | Exposes KdsNavItem + types. |
| packages/components/src/layouts/Navigation/KdsNavItem/KdsNavItem.vue | Implements nav item rendering as button/link + accessories. |
| packages/components/src/layouts/Navigation/KdsNavItem/KdsNavItem.stories.ts | Storybook stories + interaction tests for nav item behavior. |
| packages/components/src/forms/_helper/List/ListItemDivider/index.ts | Removes the old ListItemDivider export. |
| packages/components/src/forms/_helper/List/ListContainer/KdsListContainer.vue | Replaces ListItemDivider usage with KdsDivider. |
| packages/components/src/accessories/index.ts | Exports the new Divider accessory from accessories. |
| packages/components/src/accessories/Divider/index.ts | Adds the KdsDivider export. |
| packages/components/src/accessories/Divider/Divider.vue | Implements the new shared divider styling. |
| packages/components/src/accessories/Divider/Divider.stories.ts | Updates Storybook story for Divider in its new location. |
| .changeset/shaggy-kids-carry.md | Adds a changeset for releasing the new component(s). |
Comments suppressed due to low confidence (2)
packages/components/src/accessories/Divider/Divider.stories.ts:23
- The docs description still calls this an "Internal helper", but
KdsDivideris now exported frompackages/components/src/accessories/index.ts, making it part of the public accessories surface. Please update the wording to reflect whether this is intended to be public or keep it out of the public exports.
packages/components/src/accessories/Divider/Divider.stories.ts:16 - Story title is inconsistent with other components in
packages/components/src/accessories/*, which use theAccessories/...prefix (e.g.Accessories/Avatar,Accessories/Badge). Consider renaming this toAccessories/Dividerto keep Storybook navigation consistent.
thresharx
force-pushed
the
KDS-573-navigation
branch
from
442f600 to
0634228
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new KdsNavigation layout component (with KdsNavItem) and introduces a reusable KdsDivider accessory by migrating the former list item divider usage.
Changes:
- Export
Navigationfrompackages/components/src/layoutsand add newKdsNavigation+KdsNavItemimplementation with Storybook coverage. - Replace list container’s internal divider usage with the new
KdsDivideraccessory and export it fromaccessories. - Add a changeset for the new public exports.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/components/src/layouts/index.ts | Re-exports the new Navigation module from layouts. |
| packages/components/src/layouts/Navigation/types.ts | Defines public KdsNavigationProps and KdsNavigationItem types. |
| packages/components/src/layouts/Navigation/index.ts | Exports KdsNavigation and re-exports KdsNavItem + types. |
| packages/components/src/layouts/Navigation/KdsNavigation.vue | Implements navigation rendering + v-model selection tracking. |
| packages/components/src/layouts/Navigation/KdsNavigation.stories.ts | Adds Storybook stories and a play test for interaction/selection. |
| packages/components/src/layouts/Navigation/KdsNavItem/types.ts | Defines KdsNavItem props (label/link/accessories/selected/disabled). |
| packages/components/src/layouts/Navigation/KdsNavItem/index.ts | Exports KdsNavItem and its types. |
| packages/components/src/layouts/Navigation/KdsNavItem/KdsNavItem.vue | Implements nav item rendering as button/link with optional accessories. |
| packages/components/src/layouts/Navigation/KdsNavItem/KdsNavItem.stories.ts | Adds Storybook stories and play tests for KdsNavItem. |
| packages/components/src/containers/ListItem/ListItemDivider/index.ts | Removes the old divider export from the containers area. |
| packages/components/src/containers/ListContainer/KdsListContainer.vue | Switches to KdsDivider and adds local styling hook class. |
| packages/components/src/accessories/index.ts | Exports the new Divider accessory entry. |
| packages/components/src/accessories/Divider/index.ts | Exports KdsDivider. |
| packages/components/src/accessories/Divider/KdsDivider.vue | Updates divider markup/class naming to kds-divider. |
| packages/components/src/accessories/Divider/KdsDivider.stories.ts | Updates the divider Storybook story to the new component/file layout. |
| .changeset/shaggy-kids-carry.md | Adds a patch changeset documenting the new exports. |
Comments suppressed due to low confidence (2)
packages/components/src/accessories/Divider/KdsDivider.stories.ts:22
- Story title is inconsistent with other accessory stories (they use the "Accessories/..." prefix), so this will show up at the Storybook root. Also, the docs description still calls this an internal helper, but KdsDivider is now exported from
src/accessories/index.tsand is part of the public API—please update the title and description accordingly.
packages/components/src/accessories/Divider/KdsDivider.stories.ts:16 - The imported component is named
Divider, but the actual component isKdsDivider.vueand it’s exported asKdsDivider. Using the exported name in the story improves consistency and avoids confusion when searching for usages.
thresharx
force-pushed
the
KDS-573-navigation
branch
from
0634228 to
d52231c
Compare
| return { | ||
| ...props.button, | ||
| size: "xsmall", | ||
| } as KdsButtonProps; |
Collaborator
There was a problem hiding this comment.
Suggested change
| } as KdsButtonProps; | |
| } satisfies KdsButtonProps; |
Collaborator
There was a problem hiding this comment.
Question: How is the callback done for a trailing button? There is neither a click listener nor a callback prop, is it?
Contributor
Author
There was a problem hiding this comment.
that is a good point. i thought i added it but apparently not
Contributor
Author
There was a problem hiding this comment.
satisfies doesnt work here because of the spread
thresharx
force-pushed
the
KDS-573-navigation
branch
from
99cc505 to
f456662
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new KdsNavigation layout component (with KdsNavItem) to the KNIME Design System component library, and promotes the former list-item divider into a reusable accessory (KdsDivider).
Changes:
- Export new
Navigationlayout module withKdsNavigation+KdsNavItemand Storybook coverage. - Replace
ListItemDividerusage inKdsListContainerwith the newKdsDivideraccessory and remove the old divider export. - Add a changeset documenting the new component(s) and divider move.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/components/src/layouts/index.ts | Re-exports Navigation from the layouts barrel. |
| packages/components/src/layouts/Navigation/types.ts | Defines KdsNavigationProps/KdsNavigationItem types. |
| packages/components/src/layouts/Navigation/index.ts | Exports KdsNavigation, KdsNavItem, and related types. |
| packages/components/src/layouts/Navigation/KdsNavigation.vue | Implements the navigation list with v-model selection and optional separators. |
| packages/components/src/layouts/Navigation/KdsNavigation.stories.ts | Adds Storybook stories and play test for selection behavior. |
| packages/components/src/layouts/Navigation/KdsNavItem/types.ts | Defines KdsNavItemProps (label/link/accessories/selected/disabled). |
| packages/components/src/layouts/Navigation/KdsNavItem/index.ts | Exports KdsNavItem and its types. |
| packages/components/src/layouts/Navigation/KdsNavItem/KdsNavItem.vue | Implements a selectable nav item with optional badge/icon/trailing button. |
| packages/components/src/layouts/Navigation/KdsNavItem/KdsNavItem.stories.ts | Adds Storybook stories/play tests for the nav item and states. |
| packages/components/src/containers/ListItem/ListItemDivider/index.ts | Removes the old divider export. |
| packages/components/src/containers/ListContainer/KdsListContainer.vue | Switches list separators to KdsDivider and attempts local styling. |
| packages/components/src/accessories/index.ts | Re-exports new Divider accessory from the accessories barrel. |
| packages/components/src/accessories/Divider/index.ts | Exports KdsDivider. |
| packages/components/src/accessories/Divider/KdsDivider.vue | Defines the divider <hr> styling under the new component name/class. |
| packages/components/src/accessories/Divider/KdsDivider.stories.ts | Moves/updates the divider story under Accessories. |
| .changeset/shaggy-kids-carry.md | Declares a patch release for the new navigation + divider move. |
Comments suppressed due to low confidence (1)
packages/components/src/accessories/Divider/KdsDivider.stories.ts:23
KdsDivideris exported from the publicaccessoriesbarrel, but the docs description still calls it an “Internal helper”. Please update the story/docs text to describe it as a general-purpose divider (or stop exporting it publicly if it’s intended to remain internal).
…v item types, fix stories KDS-573 (Navigation)
thresharx
force-pushed
the
KDS-573-navigation
branch
from
f456662 to
01270a9
Compare
|
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.
No description provided.