feat(Tabs): rename Trigger to Button and add primitive components #398
Conversation
…-tabs-variants
…-tabs-variants
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @ZeroChoi2781, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Tabs component, renaming Tabs.Trigger to Tabs.Button and introducing new primitive components: Tabs.ListPrimitive and Tabs.IndicatorPrimitive. The Tabs.List component now internally includes Tabs.IndicatorPrimitive but allows for custom indicators via an indicatorElement prop. These changes are reflected across documentation, generated prop JSON files, and all example usages. Review comments indicate that TabsListPrimitive needs to be updated to correctly prioritize its own props (like variant, orientation, loop, activateOnFocus) over context values, and an unused TabsIndicator type definition namespace should be removed for clarity.
| export const TabsListPrimitive = forwardRef<HTMLDivElement, TabsListPrimitive.Props>( | ||
| (props, ref) => { | ||
| const { className, ...componentProps } = resolveStyles(props); | ||
| const { activateOnFocus, loop, variant, orientation } = useTabsContext(); | ||
|
|
||
| return ( | ||
| <BaseTabs.List | ||
| ref={ref} | ||
| loop={loop} | ||
| activateOnFocus={activateOnFocus} | ||
| className={clsx(styles.list({ variant, orientation }), className)} | ||
| {...componentProps} | ||
| /> | ||
| ); | ||
| }, | ||
| ); | ||
| TabsListPrimitive.displayName = 'Tabs.ListPrimitive'; |
There was a problem hiding this comment.
TabsListPrimitive 컴포넌트가 자체적으로 받는 variant, orientation, loop, activateOnFocus 등의 props를 스타일링 및 동작에 반영하지 않고 컨텍스트(useTabsContext)의 값만 사용하고 있습니다. 이로 인해 <Tabs.ListPrimitive variant="fill" /> 와 같이 props를 전달해도 의도대로 스타일이나 동작이 적용되지 않는 문제가 발생합니다. 컴포넌트에 직접 전달된 props가 컨텍스트 값보다 우선 적용되도록 수정해야 합니다.
export const TabsListPrimitive = forwardRef<HTMLDivElement, TabsListPrimitive.Props>(
(props, ref) => {
const { className, ...allProps } = resolveStyles(props);
const context = useTabsContext();
const {
variant: variantProp,
orientation: orientationProp,
loop: loopProp,
activateOnFocus: activateOnFocusProp,
...otherProps
} = allProps;
const variant = variantProp ?? context.variant;
const orientation = orientationProp ?? context.orientation;
const loop = loopProp ?? context.loop;
const activateOnFocus = activateOnFocusProp ?? context.activateOnFocus;
return (
<BaseTabs.List
ref={ref}
loop={loop}
activateOnFocus={activateOnFocus}
className={clsx(styles.list({ variant, orientation }), className)}
{...otherProps}
/>
);
},
);
TabsListPrimitive.displayName = 'Tabs.ListPrimitive';
| export namespace TabsIndicator { | ||
| export interface Props extends ComponentPropsWithoutRef<typeof BaseTabs.Indicator> {} | ||
| export interface Props extends VComponentProps<typeof BaseTabs.Indicator> {} | ||
| } |
There was a problem hiding this comment.
TabsIndicator 컴포넌트가 TabsIndicatorPrimitive로 대체되면서 해당 컴포넌트는 삭제되었지만, 타입 정의를 위한 TabsIndicator 네임스페이스는 남아있습니다. 이 네임스페이스는 현재 사용되지 않으며, index.parts.ts를 통해 외부로 노출되지도 않아 코드에 혼란을 줄 수 있습니다. 사용되지 않는 코드를 제거하여 명확성을 높이는 것이 좋습니다.
|
✅ All tests passed!
Click here if you need to update snapshots. |
| const { activateOnFocus, loop, variant, orientation } = useTabsContext(); | ||
| const { children, indicatorElement, ...componentProps } = resolveStyles(props); | ||
|
|
||
| const IndicatorElement = createSlot(indicatorElement || <TabsIndicatorPrimitive />); |
There was a problem hiding this comment.
It would be good to modify this part as well to match the PR content!
| type RootControlledProps = { | ||
| activateOnFocus?: boolean; | ||
| loop?: boolean; | ||
| disabled?: boolean; | ||
| }; |
There was a problem hiding this comment.
How about using a Pick utility type for this part? If the subcomponent's spec changes, it seems like we won't be able to catch the error at compile time.
Related Issues
N/A
Description of Changes
Component Changes (
packages/core)1. Component Renaming:
Tabs.Trigger→Tabs.Button2. New Primitive Components
Tabs.ListPrimitive: Low-level tab list container without built-in indicatorTabs.IndicatorPrimitive: Direct access to the active tab indicator element3. Enhanced
Tabs.ListindicatorElementprop for custom indicator supportTabs.ListPrimitive+Tabs.IndicatorPrimitive4. Utility Type Exports
TabsRootProps,TabsListProps,TabsListPrimitivePropsTabsButtonProps,TabsIndicatorPrimitiveProps,TabsPanelPropsDocumentation Changes (
apps/website)New Documentation
Tabs.ListPrimitiveandTabs.IndicatorPrimitiveactivateOnFocus,loop, andindicatorElementpropsNew Examples
indicatorElementpropUpdates
Tabs.Trigger→Tabs.Buttonacross all examples and documentationTabs.RootdescriptionCodemod Changes (
packages/codemod)Tabs.Trigger→Tabs.Buttonrenamingplain→fillvariant migration already supportedBreaking Changes
Migration: Use the provided codemod or manually rename all instances.
Screenshots
Checklist
Before submitting the PR, please make sure you have checked all of the following items.