[Feat] #14 - 공통 Tabs 컴포넌트 추가#49
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (2)
⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a new TypeScript Tabs UI component with Storybook stories; removes a custom issue template; simplifies the pre-commit hook to call lint-staged directly; and adds Changes
Sequence Diagram(s)(omitted — changes are component additions and small repo/config edits that do not introduce multi-actor sequential flows requiring a diagram) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 Tip Migrating from UI to YAML configuration.Use the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/shared/ui/tabs/tabs.stories.tsx`:
- Around line 54-56: The story currently snapshots initial selection with
useState(args.value) and overrides onChange, causing Controls/Actions to drift;
remove the local state and let Storybook args be the single source of truth by
returning the component with value={args.value} and onChange={args.onChange}
(i.e., replace the body of TabsWithState to return <Tabs {...args}
value={args.value} onChange={args.onChange} />), and apply the same change to
the other story wrapper functions referenced (the similar wrappers at the other
noted locations) and remove any per-story onChange: () => {} arg overrides so
the action handler is not suppressed.
In `@src/shared/ui/tabs/tabs.tsx`:
- Around line 36-37: The tab base class string itemBaseClasses currently
includes outline-none which removes the native focus ring without providing a
replacement; restore an accessible visible focus indicator by removing
outline-none or by appending a focus-visible utility (e.g., focus-visible:ring
focus-visible:ring-offset-1 focus-visible:ring-primary) to itemBaseClasses or to
the tab variant classes used in src/shared/ui/tabs/tabs.tsx so keyboard users
receive a clear focus style when a tab element (the components using
itemBaseClasses) receives focus.
- Around line 100-105: The tab buttons are currently bare <button> elements
which default to type="submit" inside forms; update the button in the Tabs
component (the element using item.id, isDisabled and onChange) to include
type="button" so clicking tabs won't submit any surrounding form (keep the
existing aria-selected, disabled and onClick logic unchanged).
- Around line 94-95: The tablist div currently uses role="tablist" but lacks
aria-orientation for vertical mode; update the Tabs component to set
aria-orientation="vertical" on the div with className={containerClasses} (the
element wrapping items.map((item) => ...)) when the component is rendered in
vertical orientation (use the component's orientation/variant prop or class
detection logic that produces containerClasses) so assistive tech is informed of
the vertical interaction model.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f7df05a2-08fb-4f74-accc-a41d4ebce3ee
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
.github/ISSUE_TEMPLATE/custom.md.husky/pre-commitpackage.jsonsrc/shared/ui/tabs/tabs.stories.tsxsrc/shared/ui/tabs/tabs.tsx
💤 Files with no reviewable changes (2)
- .husky/pre-commit
- .github/ISSUE_TEMPLATE/custom.md
| const TabsWithState = (args: React.ComponentProps<typeof Tabs>): React.ReactElement => { | ||
| const [selected, setSelected] = useState(args.value); | ||
| return <Tabs {...args} value={selected} onChange={setSelected} />; |
There was a problem hiding this comment.
Keep Storybook args as the source of truth.
useState(args.value) only snapshots the initial selection, and the per-story onChange: () => {} args suppress the action handler entirely. After the first interaction, Controls/Actions can drift from what the story is actually rendering.
Also applies to: 70-70, 81-81, 99-99, 114-114, 129-129
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/tabs/tabs.stories.tsx` around lines 54 - 56, The story
currently snapshots initial selection with useState(args.value) and overrides
onChange, causing Controls/Actions to drift; remove the local state and let
Storybook args be the single source of truth by returning the component with
value={args.value} and onChange={args.onChange} (i.e., replace the body of
TabsWithState to return <Tabs {...args} value={args.value}
onChange={args.onChange} />), and apply the same change to the other story
wrapper functions referenced (the similar wrappers at the other noted locations)
and remove any per-story onChange: () => {} arg overrides so the action handler
is not suppressed.
| const itemBaseClasses = | ||
| "relative flex items-center justify-center cursor-pointer transition-all duration-200 outline-none select-none whitespace-nowrap"; |
There was a problem hiding this comment.
Restore a visible focus indicator.
outline-none removes the native ring, but none of the variant classes add a focus-visible replacement. Keyboard users won't be able to tell which tab currently has focus.
🎯 Proposed fix
const itemBaseClasses =
- "relative flex items-center justify-center cursor-pointer transition-all duration-200 outline-none select-none whitespace-nowrap";
+ "relative flex items-center justify-center cursor-pointer transition-all duration-200 outline-none focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[color:var(--color-primary-800,`#004A9C`)] select-none whitespace-nowrap";📝 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.
| const itemBaseClasses = | |
| "relative flex items-center justify-center cursor-pointer transition-all duration-200 outline-none select-none whitespace-nowrap"; | |
| const itemBaseClasses = | |
| "relative flex items-center justify-center cursor-pointer transition-all duration-200 outline-none focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[color:var(--color-primary-800,`#004A9C`)] select-none whitespace-nowrap"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/tabs/tabs.tsx` around lines 36 - 37, The tab base class string
itemBaseClasses currently includes outline-none which removes the native focus
ring without providing a replacement; restore an accessible visible focus
indicator by removing outline-none or by appending a focus-visible utility
(e.g., focus-visible:ring focus-visible:ring-offset-1
focus-visible:ring-primary) to itemBaseClasses or to the tab variant classes
used in src/shared/ui/tabs/tabs.tsx so keyboard users receive a clear focus
style when a tab element (the components using itemBaseClasses) receives focus.
| <div className={containerClasses} role="tablist"> | ||
| {items.map((item) => { |
There was a problem hiding this comment.
Expose vertical orientation on the tablist.
role="tablist" is announced as horizontal by default. The vertical variant needs aria-orientation="vertical" so assistive tech gets the correct interaction model.
♿ Proposed fix
- <div className={containerClasses} role="tablist">
+ <div
+ className={containerClasses}
+ role="tablist"
+ aria-orientation={variant === "vertical" ? "vertical" : undefined}
+ >📝 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.
| <div className={containerClasses} role="tablist"> | |
| {items.map((item) => { | |
| <div | |
| className={containerClasses} | |
| role="tablist" | |
| aria-orientation={variant === "vertical" ? "vertical" : undefined} | |
| > | |
| {items.map((item) => { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/tabs/tabs.tsx` around lines 94 - 95, The tablist div currently
uses role="tablist" but lacks aria-orientation for vertical mode; update the
Tabs component to set aria-orientation="vertical" on the div with
className={containerClasses} (the element wrapping items.map((item) => ...))
when the component is rendered in vertical orientation (use the component's
orientation/variant prop or class detection logic that produces
containerClasses) so assistive tech is informed of the vertical interaction
model.
sebeeeen
left a comment
sebeeeen
left a comment
There was a problem hiding this comment.
컨벤션을 잘 따라주셔서 감사합니다 👍
수정 사항 코멘트 드립니다!
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/shared/ui/tabs/tabs.tsx (2)
94-95:⚠️ Potential issue | 🟡 MinorExpose vertical orientation on the tablist.
When
variant === "vertical", the tablist should setaria-orientation="vertical"so assistive tech gets the correct interaction model.♿ Proposed fix
- <div className={containerClasses} role="tablist"> + <div + className={containerClasses} + role="tablist" + aria-orientation={variant === "vertical" ? "vertical" : undefined} + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/tabs/tabs.tsx` around lines 94 - 95, The tablist div (where containerClasses and items.map are used) does not expose orientation for assistive tech; update the element that sets role="tablist" in the Tabs component to include aria-orientation="vertical" when variant === "vertical" (and default to "horizontal" or omit otherwise) so the tab interaction model is communicated correctly; locate the JSX containing <div className={containerClasses} role="tablist"> in the Tabs component and add the conditional aria-orientation attribute tied to the variant prop.
36-37:⚠️ Potential issue | 🟠 MajorRestore a visible keyboard focus style.
Line 37 removes native focus (
outline-none) but doesn’t provide afocus-visiblefallback, so keyboard users can lose focus visibility.♿ Proposed fix
const itemBaseClasses = - "relative flex items-center justify-center cursor-pointer transition-all duration-200 outline-none select-none whitespace-nowrap"; + "relative flex items-center justify-center cursor-pointer transition-all duration-200 outline-none focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[color:var(--color-primary-800,`#004A9C`)] select-none whitespace-nowrap";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/tabs/tabs.tsx` around lines 36 - 37, The shared constant itemBaseClasses removes the native focus outline ("outline-none") without providing a focus-visible fallback, which hides focus for keyboard users; update itemBaseClasses to preserve mouse-focused styles while adding a visible keyboard focus style by replacing or augmenting the existing outline-none with a focus-visible utility (e.g., add focus-visible:outline, focus-visible:outline-2, focus-visible:outline-offset-2 and a color token like focus-visible:outline-primary) so keyboard users get a clear focus ring while pointer interactions remain unaffected.
🤖 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/shared/ui/tabs/tabs.tsx`:
- Around line 109-110: Conditional rendering currently uses a truthiness check
(item.badge && ...) which prevents numeric zero badges from showing; update the
conditional in the Tabs render where item.badge is checked (the expression that
wraps the <span className="ml-[6px] inline-flex items-center
justify-center">{item.badge}</span>) to use a nullish check (item.badge != null)
so that 0 and other valid React nodes render while still excluding
null/undefined.
- Around line 100-101: The mapped tab buttons in the Tabs component are missing
a stable key which causes React warnings; in the items.map(...) return (the
<button> element) add a unique key using the TabItem id (e.g., key={item.id}) so
each mapped element has a stable identifier; locate the map over items in Tabs
(where TabItem objects are iterated) and add key={item.id} to the button JSX.
---
Duplicate comments:
In `@src/shared/ui/tabs/tabs.tsx`:
- Around line 94-95: The tablist div (where containerClasses and items.map are
used) does not expose orientation for assistive tech; update the element that
sets role="tablist" in the Tabs component to include aria-orientation="vertical"
when variant === "vertical" (and default to "horizontal" or omit otherwise) so
the tab interaction model is communicated correctly; locate the JSX containing
<div className={containerClasses} role="tablist"> in the Tabs component and add
the conditional aria-orientation attribute tied to the variant prop.
- Around line 36-37: The shared constant itemBaseClasses removes the native
focus outline ("outline-none") without providing a focus-visible fallback, which
hides focus for keyboard users; update itemBaseClasses to preserve mouse-focused
styles while adding a visible keyboard focus style by replacing or augmenting
the existing outline-none with a focus-visible utility (e.g., add
focus-visible:outline, focus-visible:outline-2, focus-visible:outline-offset-2
and a color token like focus-visible:outline-primary) so keyboard users get a
clear focus ring while pointer interactions remain unaffected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ddd01a96-b10c-4288-b530-65c2373c1d33
📒 Files selected for processing (1)
src/shared/ui/tabs/tabs.tsx
| <button | ||
| type="button" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/shared/ui/tabs/tabs.tsx | head -120Repository: ajou-industry-matching/aim-frontend
Length of output: 5378
Add a stable key prop for mapped tab items.
The <button> element returned from items.map() at line 100 is missing a key prop. Since TabItem includes a stable id field that uniquely identifies each tab, add key={item.id} to prevent React reconciliation issues and warnings.
🛠️ Proposed fix
<button
+ key={item.id}
type="button"
role="tab"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/tabs/tabs.tsx` around lines 100 - 101, The mapped tab buttons
in the Tabs component are missing a stable key which causes React warnings; in
the items.map(...) return (the <button> element) add a unique key using the
TabItem id (e.g., key={item.id}) so each mapped element has a stable identifier;
locate the map over items in Tabs (where TabItem objects are iterated) and add
key={item.id} to the button JSX.
| {item.badge && ( | ||
| <span className="ml-[6px] inline-flex items-center justify-center">{item.badge}</span> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/shared/ui/tabs/tabs.tsx | sed -n '100,120p'Repository: ajou-industry-matching/aim-frontend
Length of output: 983
🏁 Script executed:
# Also check the type definition for the item object
rg -A 10 -B 5 "interface.*Tab|type.*Tab" src/shared/ui/tabs/Repository: ajou-industry-matching/aim-frontend
Length of output: 4726
🏁 Script executed:
# Search for any usage of badge with numeric values in tests or elsewhere
rg "badge.*[0-9]|badge.*zero" src/Repository: ajou-industry-matching/aim-frontend
Length of output: 286
Fix nullish check to allow numeric zero badges.
item.badge && suppresses 0 due to truthiness. Since badge accepts React.ReactNode (which includes numbers), numeric zero badges cannot render. Use item.badge != null && to allow zero while still excluding null and undefined.
Proposed fix
- {item.badge && (
+ {item.badge != null && (
<span className="ml-[6px] inline-flex items-center justify-center">{item.badge}</span>
)}📝 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.
| {item.badge && ( | |
| <span className="ml-[6px] inline-flex items-center justify-center">{item.badge}</span> | |
| {item.badge != null && ( | |
| <span className="ml-[6px] inline-flex items-center justify-center">{item.badge}</span> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/tabs/tabs.tsx` around lines 109 - 110, Conditional rendering
currently uses a truthiness check (item.badge && ...) which prevents numeric
zero badges from showing; update the conditional in the Tabs render where
item.badge is checked (the expression that wraps the <span className="ml-[6px]
inline-flex items-center justify-center">{item.badge}</span>) to use a nullish
check (item.badge != null) so that 0 and other valid React nodes render while
still excluding null/undefined.
2 participants
🔎 What is this PR?
공통 inputBox 컴포넌트를 구현하고 Storybook 문서화를 완료
📝 Changes
📸 Screenshots (선택)
📚 Background / Context (선택)
✔ Checklist
pnpm build)pnpm lint)🙏 Request
Summary by CodeRabbit
New Features
Chores