feat(tabs): Add ellipsis for multiple tabs #4510
Conversation
🦋 Changeset detectedLatest commit: f3683d6 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. |
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe pull request introduces an enhanced overflow handling mechanism for the Tabs component. By adding state variables and methods, the implementation now supports dynamically managing tab visibility when the number of tabs exceeds the available screen space. A dropdown mechanism is introduced to display hidden tabs, allowing users to access tabs that would otherwise be out of view. The changes include adding the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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
| inline: "center", | ||
| }); | ||
| }, | ||
| [tabListProps.ref], |
There was a problem hiding this comment.
Fix useCallback dependency
The dependency array references tabListProps.ref but should reference tabList instead.
- [tabListProps.ref],
+ [tabList],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [tabListProps.ref], | |
| [tabList], |
Closes #3573
Demo:
Screen.Recording.2025-01-07.at.7.54.28.PM.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
@nextui-org/dropdownas a dependencyThis 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.