Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/underline-panels-controlled-panel-props.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
---

UnderlinePanels: Correct Panel types to exclude attributes controlled by the component.
104 changes: 58 additions & 46 deletions packages/react/src/ActionBar/ActionBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {useMergedRefs} from '../hooks'
import {createDescendantRegistry} from '../utils/descendant-registry'
import {OverflowObserverProvider} from '../internal/components/OverflowObserverProvider'
import {useIsClipped} from '../internal/hooks/useOverflowObserver'
import {mergeProps} from '../utils/mergeProps'

type ChildProps =
| {
Expand Down Expand Up @@ -332,12 +333,11 @@ function useActionBarItem(ref: React.RefObject<HTMLElement | null>, registryProp
}

export const ActionBarIconButton = forwardRef(
({disabled, onClick, ...props}: ActionBarIconButtonProps, forwardedRef) => {
({disabled, onClick, className, ...props}: ActionBarIconButtonProps, forwardedRef) => {
const ref = useRef<HTMLButtonElement>(null)
const mergedRef = useMergedRefs(forwardedRef, ref)

const {size} = React.useContext(ActionBarContext)

const {['aria-label']: ariaLabel, icon} = props

const {dataOverflowingAttr} = useActionBarItem(
Expand All @@ -364,62 +364,74 @@ export const ActionBarIconButton = forwardRef(

return (
<IconButton
aria-disabled={disabled}
ref={mergedRef}
size={size}
onClick={clickHandler}
{...props}
{...mergeProps(
{
'aria-disabled': disabled,
className,
size,
onClick: clickHandler,
},
props,
)}
variant="invisible"
data-overflowing={dataOverflowingAttr}
/>
)
},
)

export const ActionBarButton = forwardRef(({disabled, onClick, ...props}: ActionBarButtonProps, forwardedRef) => {
const ref = useRef<HTMLButtonElement>(null)
const mergedRef = useMergedRefs(forwardedRef, ref)

const {size} = React.useContext(ActionBarContext)
export const ActionBarButton = forwardRef(
({disabled, onClick, className, children, leadingVisual, ...props}: ActionBarButtonProps, forwardedRef) => {
const ref = useRef<HTMLButtonElement>(null)
const mergedRef = useMergedRefs(forwardedRef, ref)

const {children, leadingVisual} = props
const {size} = React.useContext(ActionBarContext)

const {dataOverflowingAttr} = useActionBarItem(
ref,
useMemo(
(): ChildProps => ({
type: 'action',
label: children,
// Only forward the leading visual to the overflow menu when it is a component
// that can be rendered as an icon (e.g. an octicon), matching ActionBar.IconButton.
icon: typeof leadingVisual === 'function' ? (leadingVisual as ActionBarIconButtonProps['icon']) : undefined,
disabled: !!disabled,
onClick: onClick as MouseEventHandler,
}),
[children, leadingVisual, disabled, onClick],
),
)
const {dataOverflowingAttr} = useActionBarItem(
ref,
useMemo(
(): ChildProps => ({
type: 'action',
label: children,
// Only forward the leading visual to the overflow menu when it is a component
// that can be rendered as an icon (e.g. an octicon), matching ActionBar.IconButton.
icon: typeof leadingVisual === 'function' ? (leadingVisual as ActionBarIconButtonProps['icon']) : undefined,
disabled: !!disabled,
onClick: onClick as MouseEventHandler,
}),
[children, leadingVisual, disabled, onClick],
),
)

const clickHandler = useCallback(
(event: React.MouseEvent<HTMLButtonElement>) => {
if (disabled) return
onClick?.(event)
},
[disabled, onClick],
)
const clickHandler = useCallback(
(event: React.MouseEvent<HTMLButtonElement>) => {
if (disabled) return
onClick?.(event)
},
[disabled, onClick],
)

return (
<Button
aria-disabled={disabled}
ref={mergedRef}
size={size}
onClick={clickHandler}
{...props}
variant="invisible"
data-overflowing={dataOverflowingAttr}
/>
)
})
return (
<Button
ref={mergedRef}
{...mergeProps(
{
'aria-disabled': disabled,
className,
children,
leadingVisual,
size,
onClick: clickHandler,
},
props,
)}
variant="invisible"
data-overflowing={dataOverflowingAttr}
/>
)
},
)

const ActionBarGroupContext = React.createContext<{
isOverflowing: boolean
Expand Down
13 changes: 9 additions & 4 deletions packages/react/src/Breadcrumbs/Breadcrumbs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {ResizeObserverEntry} from '../hooks/useResizeObserver'
import {useOnEscapePress} from '../hooks/useOnEscapePress'
import {useOnOutsideClick} from '../hooks/useOnOutsideClick'
import {type PolymorphicProps, fixedForwardRef} from '../utils/modern-polymorphic'
import {mergeProps} from '../utils/mergeProps'

export type BreadcrumbsProps = React.PropsWithChildren<{
/**
Expand Down Expand Up @@ -389,11 +390,15 @@ const BreadcrumbsItem = fixedForwardRef(
const {as: Component = 'a', selected, className, ...rest} = props
return (
<Component
className={clsx(className, classes.Item, selected && 'selected')}
aria-current={selected ? 'page' : undefined}
ref={ref}
data-component="Breadcrumbs.Item"
{...rest}
{...mergeProps(
{
className: clsx(classes.Item, selected && 'selected', className),
'aria-current': selected ? 'page' : undefined,
'data-component': 'Breadcrumbs.Item',
},
rest,
)}
/>
)
},
Expand Down
81 changes: 57 additions & 24 deletions packages/react/src/NavList/NavList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import HeadingComponent from '../Heading'
import visuallyHiddenClasses from '../_VisuallyHidden.module.css'
import type {FCWithSlotMarker} from '../utils/types/Slots'
import {asSlot} from '../utils/as-slot'
import {mergeProps} from '../utils/mergeProps'

type HeadingLevels = 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'

Expand Down Expand Up @@ -54,7 +55,7 @@ export type NavListProps = {
} & React.ComponentProps<'nav'>

const Root = React.forwardRef<HTMLElement, NavListProps>(
({children, 'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledby, ...props}, ref) => {
({children, className, 'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledby, ...props}, ref) => {
const [slots, childrenWithoutHeading] = useSlots(children, {
heading: Heading,
})
Expand All @@ -69,7 +70,18 @@ const Root = React.forwardRef<HTMLElement, NavListProps>(
const navLabelledby = ariaLabelledby ?? (ariaLabel ? undefined : headingId)

return (
<nav {...props} aria-label={ariaLabel} aria-labelledby={navLabelledby} ref={ref} data-component="NavList">
<nav
ref={ref}
{...mergeProps(
{
className,
'aria-label': ariaLabel,
'aria-labelledby': navLabelledby,
},
props,
)}
data-component="NavList"
>
<NavListHeadingLevelContext.Provider value={headingLevel}>
{heading ? React.cloneElement(heading, {id: headingId}) : null}
<ActionListContainerContext.Provider
Expand Down Expand Up @@ -106,16 +118,20 @@ const Heading: FCWithSlotMarker<NavListHeadingProps> = ({
}) => {
return (
<HeadingComponent
as={as}
variant="small"
className={clsx(
// Apply the visually-hidden styles directly to the heading rather than wrapping
// it in a span (a heading isn't valid phrasing content inside a span).
visuallyHidden ? visuallyHiddenClasses.InternalVisuallyHidden : navListClasses.Heading,
className,
{...mergeProps(
{
as,
variant: 'small' as const,
className: clsx(
// Apply the visually-hidden styles directly to the heading rather than wrapping
// it in a span (a heading isn't valid phrasing content inside a span).
visuallyHidden ? visuallyHiddenClasses.InternalVisuallyHidden : navListClasses.Heading,
className,
),
'data-component': 'NavList.Heading',
},
props,
)}
data-component="NavList.Heading"
{...props}
>
{children}
</HeadingComponent>
Expand Down Expand Up @@ -143,7 +159,15 @@ export type NavListItemProps<As extends React.ElementType = React.ElementType> =

const ItemComponent = fixedForwardRef(
<As extends React.ElementType = 'a'>(
{'aria-current': ariaCurrent, children, defaultOpen, tooltipText, as: Component, ...props}: NavListItemProps<As>,
{
'aria-current': ariaCurrent,
children,
defaultOpen,
tooltipText,
as: Component,
className,
...props
}: NavListItemProps<As>,
ref: React.ForwardedRef<unknown>,
) => {
const {depth} = React.useContext(SubNavContext)
Expand Down Expand Up @@ -182,13 +206,18 @@ const ItemComponent = fixedForwardRef(
return (
<InternalLinkItem
ref={ref}
as={Component}
aria-current={ariaCurrent}
active={Boolean(ariaCurrent) && ariaCurrent !== 'false'}
style={{'--subitem-depth': depth} as React.CSSProperties}
data-component="NavList.Item"
_PrivateTooltipText={tooltipText}
{...props}
{...mergeProps(
{
as: Component,
className,
'aria-current': ariaCurrent,
active: Boolean(ariaCurrent) && ariaCurrent !== 'false',
style: {'--subitem-depth': depth} as React.CSSProperties,
'data-component': 'NavList.Item',
_PrivateTooltipText: tooltipText,
},
props,
)}
>
{children}
</InternalLinkItem>
Expand Down Expand Up @@ -539,11 +568,15 @@ const GroupHeadingImpl: React.FC<NavListGroupHeadingProps> = ({as, className, ..
const resolvedAs = as ?? (headingLevel ? levelToHeadingTag(headingLevel + 1) : 'h3')
return (
<ActionList.GroupHeading
as={resolvedAs}
className={clsx(navListClasses.GroupHeading, className)}
data-component="NavList.GroupHeading"
headingWrapElement="li"
{...rest}
{...mergeProps(
{
as: resolvedAs,
className: clsx(navListClasses.GroupHeading, className),
'data-component': 'NavList.GroupHeading',
headingWrapElement: 'li',
},
rest,
)}
/>
)
}
Expand Down
Loading
Loading