feat(tabs): new tedi-ready component #555#557
Merged
mart-sessman merged 8 commits intorcfrom Apr 15, 2026
Merged
Conversation
extracted StatusIndicator from badge
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
airikej
reviewed
Mar 13, 2026
airikej
reviewed
Mar 13, 2026
airikej
reviewed
Mar 13, 2026
airikej
reviewed
Mar 13, 2026
| url: 'https://www.figma.com/design/jWiRIXhHRxwVdMSimKX2FF/TEDI-READY-2.38.59?node-id=2405-53326&m=dev', | ||
| }, | ||
| }, | ||
| argTypes: { |
Contributor
There was a problem hiding this comment.
i think these are generated automatically, you should be able to remove them
Contributor
Author
There was a problem hiding this comment.
by default it added just a string type without any selectable values. Could it be achieved without defining argTypes?
Contributor
There was a problem hiding this comment.
ah i see the problem, it does that because you define separate types e.g
export type StatusIndicatorType = 'success' | 'danger' | 'warning' | 'inactive';
export type StatusIndicatorSize = 'sm' | 'lg';
export type StatusIndicatorPosition = 'default' | 'top-right';
and then assign it like
statusIndicatorPosition: StatusIndicatorPosition
but if you add it like this:
statusIndicatorPosition: default' | 'top-right'
then it shows the selection
no idea how to fix that though
code review fixes
mart-sessman
temporarily deployed
to
github-pages
GitHub Actions
Inactive
mart-sessman
temporarily deployed
to
github-pages
GitHub Actions
Inactive
mart-sessman
temporarily deployed
to
github-pages
GitHub Actions
Inactive
airikej
approved these changes
Mar 25, 2026
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughA comprehensive Tabs component suite is added to TEDI with keyboard navigation, overflow handling (dropdown/scroll modes), and context-driven state management. A new StatusIndicator component is introduced for visual status badges. Supporting infrastructure includes a useScrollFade hook, Jest mocks for browser APIs, updated existing components (StatusBadge, ScrollFade, DropdownTrigger), and localization labels. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Trigger as TabsTrigger
participant Context as TabsContext
participant Content as TabsContent
participant Observer as ResizeObserver
User->>Trigger: Click tab (not disabled)
Trigger->>Context: setCurrentTab(id)
Context-->>Trigger: Update currentTab
Trigger-->>User: Tab appears selected
Content->>Context: Read currentTab
alt id matches currentTab
Content-->>User: Render panel
else id doesn't match
Content-->>User: Return null
end
Note over Observer: In overflow mode (dropdown)
Observer->>Observer: Detect size change
Observer->>Trigger: Calculate overflow
alt Overflow detected
Trigger-->>User: Show "More" button
User->>Trigger: Click More button
Trigger->>Content: Open dropdown
User->>Content: Select non-current tab
Content->>Context: setCurrentTab(selectedId)
Content-->>User: Close dropdown
else No overflow
Trigger-->>User: Hide "More" button
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tedi/components/overlays/dropdown/dropdown-trigger/dropdown-trigger.tsx (1)
15-21:⚠️ Potential issue | 🟠 MajorRef override risk:
children.props.refcan overwriterefs.setReference.The current spread order
{ ref: refs.setReference, ...children.props }means anyrefinchildren.propswill override the dropdown's reference setter. This would silently break Floating UI positioning if a consumer passes a trigger element with its own ref.Reverse the order so
refs.setReferencealways takes precedence:Proposed fix
return cloneElement( children, getReferenceProps({ - ref: refs.setReference, ...children.props, + ref: refs.setReference, }) );Note: If supporting both refs is a requirement, consider using a ref-merging utility (e.g.,
useMergeRefsfrom Floating UI or a custom helper).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tedi/components/overlays/dropdown/dropdown-trigger/dropdown-trigger.tsx` around lines 15 - 21, The spread order in the cloneElement call lets children.props.ref override the dropdown's reference setter; update the props passed to cloneElement/getReferenceProps so refs.setReference takes precedence (i.e., spread children.props first, then provide ref: refs.setReference) when calling cloneElement in dropdown-trigger.tsx (symbols: cloneElement, getReferenceProps, refs.setReference, children.props); if you need to preserve both refs instead, merge them with a ref-merging utility like useMergeRefs and pass the merged ref as refs.setReference.
🧹 Nitpick comments (3)
src/tedi/components/tags/status-indicator/status-indicator.tsx (1)
52-53: Simplify accessibility attributes.Since
aria-hidden="true"hides this element from assistive technology, therole="img"is effectively ignored. For purely decorative elements, you can usearia-hidden="true"alone.♻️ Suggested simplification
className={cn(...)} - role="img" aria-hidden="true" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tedi/components/tags/status-indicator/status-indicator.tsx` around lines 52 - 53, The JSX element inside the StatusIndicator component currently sets both role="img" and aria-hidden="true"; remove the redundant role="img" attribute so the decorative element is only hidden from assistive tech via aria-hidden="true". Locate the element in StatusIndicator (the tag with role="img" and aria-hidden) and delete the role prop, leaving aria-hidden="true" only to simplify accessibility.src/tedi/components/navigation/tabs/tabs-list/tabs-list.tsx (1)
77-85: Consider simplifying the ref merging.The type assertion on line 79 works but is verbose. An alternative is to use a mutable ref from the start.
♻️ Alternative using useRef with explicit null type
- const listRef = useRef<HTMLDivElement>(null); + const listRef = useRef<HTMLDivElement | null>(null); ... const mergedListRef = useCallback( (node: HTMLDivElement | null) => { - (listRef as React.MutableRefObject<HTMLDivElement | null>).current = node; + listRef.current = node; if (overflowMode === 'scroll') { scrollRef(node); } }, [overflowMode, scrollRef] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tedi/components/navigation/tabs/tabs-list/tabs-list.tsx` around lines 77 - 85, The mergedListRef callback uses a verbose type assertion on listRef; instead, make listRef a mutable ref with an explicit null type (e.g., useRef<HTMLDivElement | null>(null)) so you can assign node directly without casting, then keep mergedListRef (and its useCallback) to set listRef.current = node and call scrollRef(node) when overflowMode === 'scroll'. Update the definition of listRef where it is declared (rather than inside mergedListRef) and remove the (listRef as React.MutableRefObject...) cast to simplify the ref merging.src/tedi/components/navigation/tabs/tabs.module.scss (1)
47-47: Consider using CSS variables for hardcoded pixel values.The
48pxwidth for scroll-fade overlays is hardcoded. For consistency with the design token approach used elsewhere (e.g.,var(--borders-02),var(--tab-spacing-x)), consider using a CSS variable.♻️ Suggested change
- width: 48px; + width: var(--tab-scroll-fade-width, 48px);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tedi/components/navigation/tabs/tabs.module.scss` at line 47, Replace the hardcoded "width: 48px" in tabs.module.scss with a CSS variable to match existing design-token usage: add a new variable like --tab-scroll-fade-width (defined in :root or the module's top-level selector with a fallback of 48px) and use var(--tab-scroll-fade-width, 48px) in place of the literal 48px; this keeps consistency with variables such as --borders-02 and --tab-spacing-x and allows future theming/tokens to control the scroll-fade overlay width.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/tedi/components/navigation/tabs/tabs-content/tabs-content.tsx`:
- Around line 35-37: In TabsContent, when id prop is not provided the panel
still has role="tabpanel" but no aria-labelledby; update the component to
compute a fallback label id (e.g., use the current/active tab id from the tabs
context or prop — look for functions/values like getActiveTabId, activeTabId, or
useTabsContext) and set aria-labelledby={id ?? fallbackTabId ?? undefined} while
keeping id={id ? `${id}-panel` : undefined}; ensure the fallback value is the
corresponding tab element id so the tab↔panel ARIA linkage is preserved.
In `@src/tedi/components/navigation/tabs/tabs-helpers.ts`:
- Around line 13-17: The code assumes tabs[newIndex] exists and calls .focus() /
.scrollIntoView() without guarding against empty or invalid tab sets; update the
logic around the tabs array and index calculations (use the variables tabs,
currentIndex, newIndex) to early-return or no-op when tabs.length === 0 or
currentIndex === -1, and before invoking methods ensure typeof tabs[newIndex]
!== 'undefined' (apply the same guard to the similar block referenced at lines
34-38) so focus/scrollIntoView are only called on a valid HTMLElement.
In `@src/tedi/components/navigation/tabs/tabs-list/tabs-list.tsx`:
- Line 101: The dropdownItems array filters out the current tab (dropdownItems =
allItems.filter(item => item.id !== currentTab)), so passing active={currentTab
=== item.id} to each dropdown item is always false; remove the dead active prop
from the dropdown item JSX (or alternatively, if you intended to highlight the
current/previous tab, either stop filtering out the currentTab from
dropdownItems or change the comparison to a different selectedId variable).
Update the rendering logic around dropdownItems and the component receiving the
active prop (e.g., TabDropdownItem or similar) accordingly.
In `@src/tedi/components/navigation/tabs/tabs-trigger/tabs-trigger.tsx`:
- Around line 11-13: The JSDoc for the TabsTrigger prop `id` is inconsistent
with how `aria-controls` is generated: `TabsTrigger` currently sets
aria-controls to `${id}-panel` but the comment says it must match the
corresponding `TabsContent` id. Update one of two things: either change
`aria-controls` generation in the TabsTrigger component to use the raw `id` (so
aria-controls={id}), or change the docs/JSDoc to state that `TabsContent` should
use `${id}-panel` and keep aria-controls={\`${id}-panel\`}; ensure the naming is
consistent between `TabsTrigger` (aria-controls) and `TabsContent` (id) and
update the JSDoc on the `id` prop accordingly.
In `@src/tedi/helpers/hooks/use-scroll-fade.ts`:
- Around line 74-75: The current onScrollToStartRef.current and
onScrollToEndRef.current calls fire every frame while at the boundary; change
useScrollFade to track previous boundary state (e.g., using
atStartPrev/atEndPrev refs or state) and only invoke
onScrollToStartRef.current() when atStart becomes true and atStartPrev was
false, and only invoke onScrollToEndRef.current() when atEnd becomes true and
atEndPrev was false; after invoking update the prev refs to reflect the new
boundary state so callbacks fire only on edge transitions.
---
Outside diff comments:
In `@src/tedi/components/overlays/dropdown/dropdown-trigger/dropdown-trigger.tsx`:
- Around line 15-21: The spread order in the cloneElement call lets
children.props.ref override the dropdown's reference setter; update the props
passed to cloneElement/getReferenceProps so refs.setReference takes precedence
(i.e., spread children.props first, then provide ref: refs.setReference) when
calling cloneElement in dropdown-trigger.tsx (symbols: cloneElement,
getReferenceProps, refs.setReference, children.props); if you need to preserve
both refs instead, merge them with a ref-merging utility like useMergeRefs and
pass the merged ref as refs.setReference.
---
Nitpick comments:
In `@src/tedi/components/navigation/tabs/tabs-list/tabs-list.tsx`:
- Around line 77-85: The mergedListRef callback uses a verbose type assertion on
listRef; instead, make listRef a mutable ref with an explicit null type (e.g.,
useRef<HTMLDivElement | null>(null)) so you can assign node directly without
casting, then keep mergedListRef (and its useCallback) to set listRef.current =
node and call scrollRef(node) when overflowMode === 'scroll'. Update the
definition of listRef where it is declared (rather than inside mergedListRef)
and remove the (listRef as React.MutableRefObject...) cast to simplify the ref
merging.
In `@src/tedi/components/navigation/tabs/tabs.module.scss`:
- Line 47: Replace the hardcoded "width: 48px" in tabs.module.scss with a CSS
variable to match existing design-token usage: add a new variable like
--tab-scroll-fade-width (defined in :root or the module's top-level selector
with a fallback of 48px) and use var(--tab-scroll-fade-width, 48px) in place of
the literal 48px; this keeps consistency with variables such as --borders-02 and
--tab-spacing-x and allows future theming/tokens to control the scroll-fade
overlay width.
In `@src/tedi/components/tags/status-indicator/status-indicator.tsx`:
- Around line 52-53: The JSX element inside the StatusIndicator component
currently sets both role="img" and aria-hidden="true"; remove the redundant
role="img" attribute so the decorative element is only hidden from assistive
tech via aria-hidden="true". Locate the element in StatusIndicator (the tag with
role="img" and aria-hidden) and delete the role prop, leaving aria-hidden="true"
only to simplify accessibility.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9ea0e2e1-639b-41e2-bcc1-671944500f7a
📒 Files selected for processing (28)
jest-mocks.jssrc/community/components/tabs/tabs.stories.tsxsrc/tedi/components/misc/scroll-fade/scroll-fade.tsxsrc/tedi/components/navigation/tabs/index.tssrc/tedi/components/navigation/tabs/tabs-content/tabs-content.tsxsrc/tedi/components/navigation/tabs/tabs-context.tsxsrc/tedi/components/navigation/tabs/tabs-helpers.tssrc/tedi/components/navigation/tabs/tabs-list/tabs-list.tsxsrc/tedi/components/navigation/tabs/tabs-trigger/tabs-trigger.tsxsrc/tedi/components/navigation/tabs/tabs.module.scsssrc/tedi/components/navigation/tabs/tabs.spec.tsxsrc/tedi/components/navigation/tabs/tabs.stories.tsxsrc/tedi/components/navigation/tabs/tabs.tsxsrc/tedi/components/navigation/tabs/usage-with-router.mdxsrc/tedi/components/overlays/dropdown/dropdown-trigger/dropdown-trigger.tsxsrc/tedi/components/tags/status-badge/status-badge.module.scsssrc/tedi/components/tags/status-badge/status-badge.spec.tsxsrc/tedi/components/tags/status-badge/status-badge.tsxsrc/tedi/components/tags/status-indicator/index.tssrc/tedi/components/tags/status-indicator/status-indicator.module.scsssrc/tedi/components/tags/status-indicator/status-indicator.spec.tsxsrc/tedi/components/tags/status-indicator/status-indicator.stories.tsxsrc/tedi/components/tags/status-indicator/status-indicator.tsxsrc/tedi/helpers/hooks/use-scroll-fade.spec.tsxsrc/tedi/helpers/hooks/use-scroll-fade.tssrc/tedi/helpers/index.tssrc/tedi/index.tssrc/tedi/providers/label-provider/labels-map.ts
💤 Files with no reviewable changes (1)
- src/tedi/components/tags/status-badge/status-badge.module.scss
mart-sessman
temporarily deployed
to
github-pages
GitHub Actions
Inactive
Contributor
Author
|
@coderabbitai resolve |
✅ Actions performedComments resolved. Approval is disabled; enable |
mart-sessman
temporarily deployed
to
github-pages
GitHub Actions
Inactive
mart-sessman
temporarily deployed
to
github-pages
GitHub Actions
Inactive
mart-sessman
temporarily deployed
to
github-pages
GitHub Actions
Inactive
mart-sessman
temporarily deployed
to
github-pages
GitHub Actions
Inactive
| const dropdownItems = allItems.filter((item) => item.id !== currentTab); | ||
|
|
||
| const handleMoreSelect = (id: string) => { | ||
| if (id) { |
| top: 0; | ||
| bottom: 0; | ||
| z-index: 1; | ||
| width: 48px; |
| export interface StatusIndicatorProps { | ||
| /** | ||
| * The status type, which determines the indicator color. | ||
| * @default 'success' |
There was a problem hiding this comment.
Apostrophes are unnecessary for the default string values. At the moment it shows it as "'success'" in Storybook.
This was referenced Apr 23, 2026
github-actions Bot
pushed a commit
that referenced
this pull request
Apr 29, 2026
# [17.0.0](react-16.1.0...react-17.0.0) (2026-04-29) ### Bug Fixes * **checkbox:** invalid indicator hover border fix [#605](#605) ([#609](#609)) ([f1d62c6](f1d62c6)) * **select:** select placeholder no longer blocks context menu interactions [#584](#584) ([#585](#585)) ([e8d86ab](e8d86ab)) * **variables:** update core version, update variable names [#592](#592) ([#598](#598)) ([1f15b36](1f15b36)) ### Features * **button-group:** add mobile variant [#448](#448) ([#606](#606)) ([54dee90](54dee90)), closes [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) * **card:** add more control to borderRadius usage, add examples [#444](#444) ([#597](#597)) ([deac9db](deac9db)) * **print:** introduce PrintingProvider + context-based usePrint [#99](#99) ([#497](#497)) ([a58cb70](a58cb70)) * **spinner:** add new sizes [#586](#586) ([#589](#589)) ([fbea0c3](fbea0c3)) * **tabs:** new tedi-ready component [#555](#555) ([#557](#557)) ([9c06c51](9c06c51)) * **textarea:** add autoGrow, height and maxHeight props [#588](#588) ([#593](#593)) ([2c86740](2c86740)) * **toggle:** new TEDI-Ready component [#305](#305) ([#594](#594)) ([6f28045](6f28045)) ### BREAKING CHANGES * **print:** usePrint hook removed. Replace with usePrint from the new PrintingProvider context.
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
3 participants
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.
extracted StatusIndicator from badge
Summary by CodeRabbit
New Features
Improvements
Tests
Migration Guide — Tabs
Community → TEDI-Ready Migration
What changed: The Tabs component has been rebuilt from a flat composition model (where
TabsItemdefined both the tab label and panel content) to a compound component pattern with explicit sub-components for the tab list, triggers, and content panels. Props have been renamed to align with standard conventions (value/defaultValue/onChange).Why: The new architecture gives consumers explicit control over tab trigger rendering (icons, disabled states), supports overflow handling (dropdown or scroll), and enables router-based usage where content is rendered externally.
Before:
After:
Key changes:
@tedi-design-system/react/communityto@tedi-design-system/react/tediTabsItemholds both label and content) to compound (Tabs.Triggerfor labels,Tabs.Contentfor panels)defaultCurrentTabrenamed todefaultValuecurrentTabrenamed tovalue(controlled mode)onTabChangerenamed toonChangearia-labelledbymoved fromTabsroot toTabs.ListhideNavOnPrintreplaced byprintVisibilityonTabs.List, with an additional'print-only'option ('show' | 'hide' | 'print-only')TabsItem.labelprop removed — the tab label is now thechildrenofTabs.TriggerTabsItem.padding/TabsItem.backgroundremoved — content panels no longer wrap in aCard. Style the content yourself if neededoverflowModeonTabs.List— set to'dropdown'(default) or'scroll'to handle tabs that don't fiticonprop onTabs.Trigger— display an icon before the tab labeldisabledprop onTabs.Trigger— disable individual tabsidonTabs.Content— omit theidto always render content (useful for router-based tab navigation)Mapping Table
TabsTabsTabsItemTabs.Trigger+Tabs.ContentTabsItem.labelTabs.TriggerchildrenTabsItem.idTabs.Trigger.id+Tabs.Content.ididmust be set on bothTabsItem.paddingTabsItem.backgroundTabsNavTabs.ListTabsNavItemTabs.Trigger<button>Tabs.aria-labelledbyTabs.List.aria-labelledbyTabs.hideNavOnPrintTabs.List.printVisibilityTabs.List.overflowMode'dropdown'or'scroll'Tabs.Trigger.iconTabs.Trigger.disabledNew Component — StatusIndicator
Import:
Props:
type'success' | 'danger' | 'warning' | 'inactive''success'size'sm' | 'lg''sm'hasBorderbooleanfalseposition'default' | 'top-right''default'classNamestringThis is an additive change — no migration action required.