-
Notifications
You must be signed in to change notification settings - Fork 638
Remove sx from UnderlinePanels #6874
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9548fcc
7e4132e
54c09fb
64d1fc9
82a68db
da12e6e
a868c4d
842347d
aec771a
b4bd28b
891bcf8
6587d22
24c9bdf
163e7bd
4962e4f
8850160
0578bcb
eb95b53
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@primer/react": major | ||
--- | ||
|
||
Remove sx from UnderlinePanels |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,36 +1,37 @@ | ||||||
// Used for UnderlineNav and UnderlinePanels components | ||||||
|
||||||
import type React from 'react' | ||||||
import {forwardRef, type FC, type PropsWithChildren} from 'react' | ||||||
import React from 'react' | ||||||
import {type ForwardedRef, forwardRef, type FC, type PropsWithChildren, type ElementType} from 'react' | ||||||
import {isElement} from 'react-is' | ||||||
import type {IconProps} from '@primer/octicons-react' | ||||||
import CounterLabel from '../../CounterLabel' | ||||||
import {type SxProp} from '../../sx' | ||||||
import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../../utils/polymorphic' | ||||||
|
||||||
import classes from './UnderlineTabbedInterface.module.css' | ||||||
import {clsx} from 'clsx' | ||||||
import {BoxWithFallback} from './BoxWithFallback' | ||||||
|
||||||
// The gap between the list items. It is a constant because the gap is used to calculate the possible number of items that can fit in the container. | ||||||
export const GAP = 8 | ||||||
|
||||||
type UnderlineWrapperProps = { | ||||||
type UnderlineWrapperProps<As extends React.ElementType> = { | ||||||
slot?: string | ||||||
as?: React.ElementType | ||||||
as?: As | ||||||
className?: string | ||||||
ref?: React.Ref<unknown> | ||||||
} & SxProp | ||||||
ref?: React.Ref<HTMLElement> | ||||||
} | ||||||
|
||||||
export const UnderlineWrapper = forwardRef( | ||||||
({children, className, ...rest}: PropsWithChildren<UnderlineWrapperProps>, forwardedRef) => { | ||||||
return ( | ||||||
<BoxWithFallback className={clsx(classes.UnderlineWrapper, className)} ref={forwardedRef} {...rest}> | ||||||
{children} | ||||||
</BoxWithFallback> | ||||||
) | ||||||
}, | ||||||
) | ||||||
export const UnderlineWrapper = forwardRef((props, ref) => { | ||||||
const {children, className, as: Component = 'nav', ...rest} = props | ||||||
return ( | ||||||
<Component | ||||||
className={clsx(classes.UnderlineWrapper, className)} | ||||||
ref={ref as ForwardedRef<HTMLDivElement>} | ||||||
{...rest} | ||||||
> | ||||||
{children} | ||||||
</Component> | ||||||
) | ||||||
}) as PolymorphicForwardRefComponent<ElementType, UnderlineWrapperProps<ElementType>> | ||||||
|
||||||
export const UnderlineItemList = forwardRef(({children, ...rest}: PropsWithChildren, forwardedRef) => { | ||||||
return ( | ||||||
|
@@ -44,51 +45,38 @@ export const LoadingCounter = () => { | |||||
return <span className={classes.LoadingCounter} /> | ||||||
} | ||||||
|
||||||
export type UnderlineItemProps = { | ||||||
as?: React.ElementType | 'a' | 'button' | ||||||
export type UnderlineItemProps<As extends React.ElementType> = { | ||||||
as?: As | 'a' | 'button' | ||||||
className?: string | ||||||
iconsVisible?: boolean | ||||||
loadingCounters?: boolean | ||||||
counter?: number | string | ||||||
icon?: FC<IconProps> | React.ReactElement | ||||||
id?: string | ||||||
ref?: React.Ref<unknown> | ||||||
} & SxProp | ||||||
} & React.ComponentPropsWithoutRef<As extends 'a' ? 'a' : As extends 'button' ? 'button' : As> | ||||||
|
||||||
export const UnderlineItem = forwardRef( | ||||||
( | ||||||
{ | ||||||
as = 'a', | ||||||
children, | ||||||
counter, | ||||||
icon: Icon, | ||||||
iconsVisible, | ||||||
loadingCounters, | ||||||
className, | ||||||
...rest | ||||||
}: PropsWithChildren<UnderlineItemProps>, | ||||||
forwardedRef, | ||||||
) => { | ||||||
return ( | ||||||
<BoxWithFallback ref={forwardedRef} as={as} className={clsx(classes.UnderlineItem, className)} {...rest}> | ||||||
{iconsVisible && Icon && <span data-component="icon">{isElement(Icon) ? Icon : <Icon />}</span>} | ||||||
{children && ( | ||||||
<span data-component="text" data-content={children}> | ||||||
{children} | ||||||
export const UnderlineItem = React.forwardRef((props, ref) => { | ||||||
const {as: Component = 'a', children, counter, icon: Icon, iconsVisible, loadingCounters, className, ...rest} = props | ||||||
Comment on lines
+59
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nitpick] The forwardRef should include proper TypeScript generics and parameter names for better type inference and debugging. Consider using the same pattern as UnderlineWrapper with explicit generic parameters. See below for a potential fix:
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
return ( | ||||||
<Component {...rest} ref={ref} className={clsx(classes.UnderlineItem, className)}> | ||||||
{iconsVisible && Icon && <span data-component="icon">{isElement(Icon) ? Icon : <Icon />}</span>} | ||||||
{children && ( | ||||||
<span data-component="text" data-content={children}> | ||||||
{children} | ||||||
</span> | ||||||
)} | ||||||
{counter !== undefined ? ( | ||||||
loadingCounters ? ( | ||||||
<span data-component="counter"> | ||||||
<LoadingCounter /> | ||||||
</span> | ||||||
)} | ||||||
{counter !== undefined ? ( | ||||||
loadingCounters ? ( | ||||||
<span data-component="counter"> | ||||||
<LoadingCounter /> | ||||||
</span> | ||||||
) : ( | ||||||
<span data-component="counter"> | ||||||
<CounterLabel>{counter}</CounterLabel> | ||||||
</span> | ||||||
) | ||||||
) : null} | ||||||
</BoxWithFallback> | ||||||
) | ||||||
}, | ||||||
) as PolymorphicForwardRefComponent<'a', UnderlineItemProps> | ||||||
) : ( | ||||||
<span data-component="counter"> | ||||||
<CounterLabel>{counter}</CounterLabel> | ||||||
</span> | ||||||
) | ||||||
) : null} | ||||||
</Component> | ||||||
) | ||||||
}) as PolymorphicForwardRefComponent<ElementType, UnderlineItemProps<ElementType>> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The type assertion uses
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
import { | ||
UnderlinePanels as PrimerUnderlinePanels, | ||
type UnderlinePanelsProps as PrimerUnderlinePanelsProps, | ||
type UnderlinePanelsPanelProps as PrimerUnderlinePanelsPanelProps, | ||
type UnderlinePanelsTabProps as PrimerUnderlinePanelsTabProps, | ||
} from '@primer/react/experimental' | ||
import styled from 'styled-components' | ||
import {sx, type SxProp} from '../sx' | ||
|
||
type UnderlinePanelsProps = PrimerUnderlinePanelsProps & SxProp | ||
|
||
const StyledUnderlinePanels = styled(PrimerUnderlinePanels).withConfig<UnderlinePanelsProps>({ | ||
shouldForwardProp: prop => prop !== 'sx', | ||
})` | ||
${sx} | ||
` | ||
|
||
// @ts-ignore forwardedAs is valid here but I don't know how to fix the typescript error | ||
const UnderlinePanelsImpl = ({as, ...props}: UnderlinePanelsProps) => ( | ||
<StyledUnderlinePanels forwardedAs={as} {...props} /> | ||
) | ||
|
||
UnderlinePanelsImpl.displayName = 'UnderlinePanels' | ||
|
||
type UnderlinePanelsPanelProps = PrimerUnderlinePanelsPanelProps & SxProp | ||
|
||
const StyledUnderlinePanelsPanel = styled(PrimerUnderlinePanels.Panel).withConfig<UnderlinePanelsPanelProps>({ | ||
shouldForwardProp: prop => prop !== 'sx', | ||
})` | ||
${sx} | ||
` | ||
|
||
// @ts-ignore forwardedAs is valid here but I don't know how to fix the typescript error | ||
const UnderlinePanelsPanel = ({as, ...props}: UnderlinePanelsPanelProps) => { | ||
return <StyledUnderlinePanelsPanel forwardedAs={as} {...props} /> | ||
} | ||
|
||
UnderlinePanelsPanel.displayName = 'UnderlinePanels.Panel' | ||
|
||
type UnderlinePanelsTabProps = PrimerUnderlinePanelsTabProps & SxProp | ||
|
||
const StyledUnderlinePanelsTab = styled(PrimerUnderlinePanels.Tab).withConfig<UnderlinePanelsTabProps>({ | ||
shouldForwardProp: prop => prop !== 'sx', | ||
})` | ||
${sx} | ||
` | ||
// @ts-ignore forwardedAs is valid here but I don't know how to fix the typescript error | ||
const UnderlinePanelsTab = ({as, ...props}: UnderlinePanelsTabProps) => ( | ||
<StyledUnderlinePanelsTab forwardedAs={as} {...props} /> | ||
) | ||
|
||
UnderlinePanelsTab.displayName = 'UnderlinePanels.Tab' | ||
|
||
const UnderlinePanels = Object.assign(UnderlinePanelsImpl, { | ||
Tab: UnderlinePanelsTab, | ||
Panel: UnderlinePanelsPanel, | ||
}) | ||
|
||
export {UnderlinePanels, type UnderlinePanelsProps, type UnderlinePanelsTabProps, type UnderlinePanelsPanelProps} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,16 @@ | ||
export {Dialog, type DialogProps} from './components/Dialog' | ||
|
||
export { | ||
PageHeader, | ||
type PageHeaderProps, | ||
type PageHeaderActionsProps, | ||
type PageHeaderTitleProps, | ||
} from './components/PageHeader' | ||
export {Table, Tooltip, UnderlinePanels} from '@primer/react/experimental' | ||
|
||
export { | ||
UnderlinePanels, | ||
type UnderlinePanelsProps, | ||
type UnderlinePanelsTabProps, | ||
type UnderlinePanelsPanelProps, | ||
} from './components/UnderlinePanels' | ||
export {Table, Tooltip} from '@primer/react/experimental' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😵💫