feat(tabs): Add ellipsis for multiple tabs #4510
Conversation
🦋 Changeset detectedLatest commit: 26cdc42 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
|
@deepansh946 is attempting to deploy a commit to the NextUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe pull request involves a comprehensive update of the Tabs component, including a renaming of the package from Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/components/tabs/src/tabs.tsx (3)
43-77: Consider optimizing overflow calculationsThe overflow detection logic is correct but could benefit from performance optimizations:
- Consider debouncing the scroll event handler
- Cache DOM measurements where possible to reduce layout thrashing
+ import {debounce} from "@nextui-org/shared-utils"; + const checkOverflow = useCallback( - () => { + debounce(() => { if (!tabList) return; // ... rest of the function - }, + }, 150), [state.collection, tabListProps.ref] );
79-94: Clean scroll implementation!The scroll behavior is well-implemented. Consider adding error boundaries or logging for cases where the tab element isn't found.
const scrollToTab = useCallback( (key: string) => { if (!tabList) return; const tabElement = tabList.querySelector(`[data-key="${key}"]`); - if (!tabElement) return; + if (!tabElement) { + console.warn(`Tab with key "${key}" not found`); + return; + }
139-175: Enhance visual feedback for overflow stateThe overflow handling is well-implemented with proper accessibility. Consider adding:
- Visual indicator for scroll shadows when content is overflowing
- Active state styling for the ellipsis button when dropdown is open
<button aria-label="Show more tabs" - className="flex-none flex items-center justify-center w-10 h-8 ml-1 hover:bg-default-100 rounded-small transition-colors" + className={clsx( + "flex-none flex items-center justify-center w-10 h-8 ml-1 hover:bg-default-100 rounded-small transition-colors", + isDropdownOpen && "bg-default-100" + )} >
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/components/tabs/package.json(1 hunks)packages/components/tabs/src/tabs.tsx(3 hunks)
🔇 Additional comments (3)
packages/components/tabs/src/tabs.tsx (3)
1-6: Well-structured state management and imports!The new imports and state variables are well-organized and properly typed for managing tab overflow functionality.
Also applies to: 34-36
96-105: LGTM! Well-structured tab selection handlerThe handler properly manages state updates and scroll behavior.
177-186: LGTM! Tab panel rendering maintainedThe tab panel rendering properly maintains existing functionality while integrating with the new overflow handling.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/components/tabs/src/tabs.tsx (1)
154-155:⚠️ Potential issueAdd cleanup for scroll event listener
The scroll event listener should be cleaned up to prevent memory leaks.
🧹 Nitpick comments (2)
packages/components/tabs/src/tabs.tsx (2)
43-77: Consider optimizing overflow detection performanceThe current implementation performs DOM measurements in a loop, which could impact performance with many tabs.
Consider these optimizations:
const checkOverflow = useCallback(() => { if (!tabList) return; const isOverflowing = tabList.scrollWidth > tabList.clientWidth; setShowOverflow(isOverflowing); if (!isOverflowing) { setHiddenTabs([]); return; } const tabs = [...state.collection]; const hiddenTabsList: Array<{key: string; title: string}> = []; const {left: containerLeft, right: containerRight} = tabList.getBoundingClientRect(); + // Cache all tab elements and their measurements upfront + const tabElements = tabs.map(item => ({ + item, + element: tabList.querySelector(`[data-key="${item.key}"]`), + })); - tabs.forEach((item) => { + tabElements.forEach(({item, element}) => { - const tabElement = tabList.querySelector(`[data-key="${item.key}"]`); - if (!tabElement) return; + if (!element) return; - const {left: tabLeft, right: tabRight} = tabElement.getBoundingClientRect(); + const {left: tabLeft, right: tabRight} = element.getBoundingClientRect();
170-177: Consider handling loading stateThe dropdown menu might benefit from a loading state indicator while fetching tab content.
<DropdownMenu aria-label="Hidden tabs" onAction={(key) => handleTabSelect(key as string)} > + {hiddenTabs.length === 0 && ( + <DropdownItem isDisabled>No hidden tabs</DropdownItem> + )} {hiddenTabs.map((tab) => ( <DropdownItem key={tab.key}>{tab.title}</DropdownItem> ))} </DropdownMenu>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/curly-snails-yell.md(1 hunks)packages/components/tabs/package.json(1 hunks)packages/components/tabs/src/tabs.tsx(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/curly-snails-yell.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/tabs/package.json
🔇 Additional comments (2)
packages/components/tabs/src/tabs.tsx (2)
34-36: LGTM! Well-structured state managementThe state variables are well-defined with clear purposes:
showOverflowfor controlling overflow visibilityhiddenTabsfor managing tabs outside viewportisDropdownOpenfor dropdown state
162-168: LGTM! Excellent accessibility implementationGreat attention to accessibility with:
- Proper aria-label
- Screen reader text
- Semantic button element
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
.changeset/curly-snails-yell.md(1 hunks)packages/components/tabs/src/tabs.tsx(3 hunks)packages/components/tabs/stories/tabs.stories.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .changeset/curly-snails-yell.md
🔇 Additional comments (7)
packages/components/tabs/stories/tabs.stories.tsx (2)
84-92: LGTM! Well-structured template for testing overflow scenarios.The implementation efficiently generates multiple tabs using Array spread and map, providing good test coverage for the new overflow functionality.
294-301: LGTM! Story export follows established patterns.The MultipleTabs story is properly configured with the solid variant, maintaining consistency with other stories.
packages/components/tabs/src/tabs.tsx (5)
1-6: LGTM! Well-organized imports and state management.The new imports and state variables are properly structured for managing tab overflow functionality.
Also applies to: 34-36
79-94: Fix useCallback dependency.The dependency array references tabListProps.ref but should reference tabList instead.
96-105: LGTM! Comprehensive tab selection handling.The function properly manages state updates, dropdown visibility, and tab scrolling.
107-123: LGTM! Proper effect management with cleanup.The effects are well-implemented with:
- Appropriate overflow style management
- Debounced resize handling
- Proper cleanup
143-190: LGTM! Accessible and well-structured overflow UI.The implementation:
- Properly handles overflow with dynamic width adjustment
- Includes appropriate ARIA labels
- Maintains good accessibility with screen reader support
deepansh946
force-pushed
the
feat/dropdown-in-tabs
branch
from
8d3a500 to
2df0fdd
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/components/tabs/src/tabs.tsx (2)
43-77: Optimize overflow detection performance.The overflow detection logic is well-implemented but could be optimized:
- Consider memoizing the hiddenTabsList calculation
- The DOM queries could be batched for better performance
const checkOverflow = useCallback(() => { if (!tabList) return; const isOverflowing = tabList.scrollWidth > tabList.clientWidth; setShowOverflow(isOverflowing); if (!isOverflowing) { setHiddenTabs([]); return; } const tabs = [...state.collection]; - const hiddenTabsList: Array<{key: string; title: string}> = []; + // Batch DOM queries + const tabElements = tabs.map(item => ({ + item, + element: tabList.querySelector(`[data-key="${item.key}"]`) + })); + + const {left: containerLeft, right: containerRight} = tabList.getBoundingClientRect(); + + const hiddenTabsList = tabElements + .filter(({element}) => element) + .filter(({element}) => { + const {left: tabLeft, right: tabRight} = element!.getBoundingClientRect(); + return tabRight > containerRight || tabLeft < containerLeft; + }) + .map(({item}) => ({ + key: String(item.key), + title: item.textValue || "", + })); - const {left: containerLeft, right: containerRight} = tabList.getBoundingClientRect(); - - tabs.forEach((item) => { - const tabElement = tabList.querySelector(`[data-key="${item.key}"]`); - - if (!tabElement) return; - - const {left: tabLeft, right: tabRight} = tabElement.getBoundingClientRect(); - const isHidden = tabRight > containerRight || tabLeft < containerLeft; - - if (isHidden) { - hiddenTabsList.push({ - key: String(item.key), - title: item.textValue || "", - }); - } - }); setHiddenTabs(hiddenTabsList); }, [state.collection, tabList]);
154-155: Optimize scroll event handling.Consider debouncing the scroll event handler to improve performance.
+const debouncedCheckOverflow = debounce(checkOverflow, 100); + <Component {...tabListProps} className={clsx( "relative flex overflow-x-auto scrollbar-hide", showOverflow ? "w-[calc(100%-32px)]" : "w-full", tabListProps.className, )} data-has-overflow={dataAttr(showOverflow)} - onScroll={checkOverflow} + onScroll={debouncedCheckOverflow} >
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/components/tabs/src/tabs.tsx(3 hunks)
🔇 Additional comments (6)
packages/components/tabs/src/tabs.tsx (6)
1-1: LGTM! Clean import organization.The new imports are well-organized and properly scoped for the ellipsis functionality.
Also applies to: 4-6
34-36: LGTM! Well-structured state management.The state variables are appropriately typed and initialized for managing overflow tabs and dropdown state.
96-105: LGTM! Well-implemented tab selection handling.The handleTabSelect function properly combines selection, dropdown state management, and scrolling.
143-179: LGTM! Accessible overflow implementation.The overflow UI implementation is well-structured with proper accessibility attributes:
- ARIA labels for the dropdown trigger and menu
- Screen reader text for the ellipsis button
- Proper keyboard navigation support through the dropdown component
107-111:⚠️ Potential issueFix useEffect dependency.
The effect's dependency array should reference
tabListinstead oftabListProps.ref.useEffect(() => { if (!tabList) return; tabList.style.overflowX = isDropdownOpen ? "hidden" : "auto"; -}, [isDropdownOpen, tabListProps.ref]); +}, [isDropdownOpen, tabList]);Likely invalid or redundant comment.
79-94:⚠️ Potential issueFix useCallback dependency in scrollToTab.
The dependency array incorrectly references
tabListProps.refinstead oftabList.const scrollToTab = useCallback( (key: string) => { if (!tabList) return; const tabElement = tabList.querySelector(`[data-key="${key}"]`); if (!tabElement) return; tabElement.scrollIntoView({ behavior: "smooth", block: "nearest", inline: "center", }); }, - [tabListProps.ref], + [tabList], );Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/components/tabs/package.json (1)
60-68:⚠️ Potential issueMove dropdown to runtime dependencies.
The
@nextui-org/dropdownpackage is used directly in the component's implementation and should be moved fromdevDependenciestodependencies.packages/components/tabs/src/tabs.tsx (1)
79-94:⚠️ Potential issueFix useCallback dependency array.
The dependency array references
tabListProps.refbut should referencetabListinstead.
🧹 Nitpick comments (1)
packages/components/tabs/src/tabs.tsx (1)
43-77: Consider performance optimizations for overflow detection.The overflow detection logic is sound but could benefit from performance improvements:
- Cache DOM measurements
- Use ResizeObserver instead of scroll event
- Add error boundaries for DOM operations
const checkOverflow = useCallback(() => { if (!tabList) return; + try { + const measurements = new Map(); const isOverflowing = tabList.scrollWidth > tabList.clientWidth; setShowOverflow(isOverflowing); if (!isOverflowing) { setHiddenTabs([]); return; } const tabs = [...state.collection]; const hiddenTabsList: Array<{key: string; title: string}> = []; const {left: containerLeft, right: containerRight} = tabList.getBoundingClientRect(); tabs.forEach((item) => { const tabElement = tabList.querySelector(`[data-key="${item.key}"]`); if (!tabElement) return; + const cachedMeasurement = measurements.get(item.key); + const {left: tabLeft, right: tabRight} = cachedMeasurement || tabElement.getBoundingClientRect(); + if (!cachedMeasurement) { + measurements.set(item.key, {left: tabLeft, right: tabRight}); + } - const {left: tabLeft, right: tabRight} = tabElement.getBoundingClientRect(); const isHidden = tabRight > containerRight || tabLeft < containerLeft; if (isHidden) { hiddenTabsList.push({ key: String(item.key), title: item.textValue || "", }); } }); setHiddenTabs(hiddenTabsList); + } catch (error) { + console.error('Error checking overflow:', error); + } }, [state.collection, tabList]);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
packages/components/tabs/package.json(4 hunks)packages/components/tabs/src/tabs.tsx(3 hunks)
🔇 Additional comments (2)
packages/components/tabs/package.json (1)
2-3: Verify version bump aligns with semantic versioning.The version bump from 2.2.7 to 2.2.8 indicates a patch change, but adding ellipsis functionality for tabs is a feature addition that should warrant a minor version bump (2.3.0).
packages/components/tabs/src/tabs.tsx (1)
34-36: LGTM! State management looks good.The state variables are well-defined and serve clear purposes:
showOverflowcontrols ellipsis visibilityhiddenTabstracks tabs not in viewisDropdownOpenmanages dropdown state
|
@wingkwong Changes are done. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
- documentation is missing
- the "show more" button should be customized (e.g. icon, aria-label etc) by users.
- in this PR's storybook, clicking tab 25 doesn't scroll to the tabs.
- may also check some extreme cases like having 1000 tabs to see if the dropdown is scrollable or not.
|
@wingkwong I have fixed the bug & applied the desired changes. |
|
@deepansh946 BIG THANKS for you work on this one 👏👏 Looking forward for this being released. |
@heroui/accordion
@heroui/alert
@heroui/autocomplete
@heroui/avatar
@heroui/badge
@heroui/breadcrumbs
@heroui/button
@heroui/calendar
@heroui/card
@heroui/checkbox
@heroui/chip
@heroui/code
@heroui/date-input
@heroui/date-picker
@heroui/divider
@heroui/drawer
@heroui/dropdown
@heroui/form
@heroui/image
@heroui/input
@heroui/input-otp
@heroui/kbd
@heroui/link
@heroui/listbox
@heroui/menu
@heroui/modal
@heroui/navbar
@heroui/number-input
@heroui/pagination
@heroui/popover
@heroui/progress
@heroui/radio
@heroui/ripple
@heroui/scroll-shadow
@heroui/select
@heroui/skeleton
@heroui/slider
@heroui/snippet
@heroui/spacer
@heroui/spinner
@heroui/switch
@heroui/table
@heroui/tabs
@heroui/toast
@heroui/tooltip
@heroui/user
@heroui/react
@heroui/system
@heroui/system-rsc
@heroui/theme
@heroui/use-aria-accordion
@heroui/use-aria-accordion-item
@heroui/use-aria-button
@heroui/use-aria-link
@heroui/use-aria-modal-overlay
@heroui/use-aria-multiselect
@heroui/use-callback-ref
@heroui/use-clipboard
@heroui/use-data-scroll-overflow
@heroui/use-disclosure
@heroui/use-draggable
@heroui/use-image
@heroui/use-infinite-scroll
@heroui/use-intersection-observer
@heroui/use-is-mobile
@heroui/use-is-mounted
@heroui/use-measure
@heroui/use-pagination
@heroui/use-real-shape
@heroui/use-ref-state
@heroui/use-resize
@heroui/use-safe-layout-effect
@heroui/use-scroll-position
@heroui/use-ssr
@heroui/use-theme
@heroui/use-update-effect
@heroui/aria-utils
@heroui/dom-animation
@heroui/framer-utils
@heroui/react-rsc-utils
@heroui/react-utils
@heroui/shared-icons
@heroui/shared-utils
@heroui/stories-utils
@heroui/test-utils
commit: |
Closes #3573
Demo:
Screen.Recording.2025-02-11.at.2.32.22.AM.mov
📝 Description
This PR introduces ellipsis support for the Tab component enabling easy access to tabs which are not in the viewport. The implementation uses DOM elements to calculate elements out of the screen & show ellipsis accordingly.
⛳️ Current behavior (updates)
Currently all the tabs can be viewed by scrolling horizontally which isn't a great UX for multiple tabs.
🚀 New behavior
Added ellipsis menu at the end of the tab list, so that all other tabs can be accessed by that.
💣 Is this a breaking change (Yes/No):
No
📝 Additional Information
Summary by CodeRabbit
New Features
Dependencies
@heroui/tabsand version to2.2.8.@nextui-orgdependencies with@heroui.This update introduces a more robust and user-friendly Tabs component that dynamically manages tab visibility and provides an intuitive way to access hidden tabs through a dropdown menu.