diff --git a/.changeset/light-schools-wish.md b/.changeset/light-schools-wish.md new file mode 100644 index 00000000000..7368a2e3133 --- /dev/null +++ b/.changeset/light-schools-wish.md @@ -0,0 +1,5 @@ +--- +'@primer/react': major +--- + +Removes `Box` component usage and `sx` prop from the `Link` component, Storybook stories, and a .figma.tsx file diff --git a/packages/react/src/Hidden/Hidden.examples.stories.tsx b/packages/react/src/Hidden/Hidden.examples.stories.tsx index 2ee71b07234..a9cbda74ad9 100644 --- a/packages/react/src/Hidden/Hidden.examples.stories.tsx +++ b/packages/react/src/Hidden/Hidden.examples.stories.tsx @@ -61,7 +61,7 @@ export const PullRequestPage = () => ( Open - + broccolinisoup {' '} wants to merge 3 commits into diff --git a/packages/react/src/Link/Link.docs.json b/packages/react/src/Link/Link.docs.json index a340feb72bc..8887776364e 100644 --- a/packages/react/src/Link/Link.docs.json +++ b/packages/react/src/Link/Link.docs.json @@ -58,11 +58,6 @@ "name": "as", "type": "React.ElementType", "defaultValue": "\"a\"" - }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true } ], "subcomponents": [] diff --git a/packages/react/src/Link/Link.tsx b/packages/react/src/Link/Link.tsx index bba00e98cb6..59d218173c1 100644 --- a/packages/react/src/Link/Link.tsx +++ b/packages/react/src/Link/Link.tsx @@ -1,13 +1,12 @@ import {clsx} from 'clsx' -import React, {forwardRef, useEffect} from 'react' +import React, {useEffect, type ForwardedRef, type ElementRef} from 'react' import {useRefObjectAsForwardedRef} from '../hooks' -import type {SxProp} from '../sx' import classes from './Link.module.css' -import Box from '../Box' import type {ComponentProps} from '../utils/types' -import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' +import {type PolymorphicProps, fixedForwardRef} from '../utils/modern-polymorphic' -type StyledLinkProps = { +type StyledLinkProps = { + as?: As /** @deprecated use CSS modules to style hover color */ hoverColor?: string muted?: boolean @@ -15,11 +14,15 @@ type StyledLinkProps = { underline?: boolean // Link inside a text block inline?: boolean -} & SxProp +} -const Link = forwardRef(({as: Component = 'a', className, inline, underline, hoverColor, ...props}, forwardedRef) => { - const innerRef = React.useRef(null) - useRefObjectAsForwardedRef(forwardedRef, innerRef) +export const UnwrappedLink = ( + props: PolymorphicProps, + ref: ForwardedRef, +) => { + const {as: Component = 'a', className, inline, underline, hoverColor, ...restProps} = props + const innerRef = React.useRef>(null) + useRefObjectAsForwardedRef(ref, innerRef) if (__DEV__) { /** @@ -46,37 +49,23 @@ const Link = forwardRef(({as: Component = 'a', className, inline, underline, hov }, [innerRef]) } - if (props.sx) { - return ( - - ) - } - return ( ) -}) as PolymorphicForwardRefComponent<'a', StyledLinkProps> +} + +const LinkComponent = fixedForwardRef(UnwrappedLink) -Link.displayName = 'Link' +const Link = Object.assign(LinkComponent, {displayName: 'Link'}) export type LinkProps = ComponentProps export default Link diff --git a/packages/react/src/Link/__tests__/Link.test.tsx b/packages/react/src/Link/__tests__/Link.test.tsx index e43fe5737c4..142ab485496 100644 --- a/packages/react/src/Link/__tests__/Link.test.tsx +++ b/packages/react/src/Link/__tests__/Link.test.tsx @@ -18,11 +18,6 @@ describe('Link', () => { expect(container.firstChild).toHaveStyle('--fgColor-accent: #0969da') }) - it('respects the "sx" prop', () => { - const {container} = render() - expect(container.firstChild).toHaveStyle('font-style: italic') - }) - it('applies button styles when rendering a button element', () => { const {container} = render() expect((container.firstChild as Element).tagName).toBe('BUTTON') @@ -33,12 +28,6 @@ describe('Link', () => { expect(container.firstChild).toHaveAttribute('data-muted', 'true') }) - it('respects the "sx" prop when "muted" prop is also passed', () => { - const {container} = render() - expect(container.firstChild).toHaveAttribute('data-muted', 'true') - expect(container.firstChild).toHaveStyle('color: rgb(89, 99, 110)') - }) - it('logs a warning when trying to render invalid "as" prop', () => { const consoleSpy = vi.spyOn(globalThis.console, 'error').mockImplementation(() => {}) diff --git a/packages/react/src/Overlay/Overlay.features.stories.module.css b/packages/react/src/Overlay/Overlay.features.stories.module.css index b8fedfc4532..590996551c4 100644 --- a/packages/react/src/Overlay/Overlay.features.stories.module.css +++ b/packages/react/src/Overlay/Overlay.features.stories.module.css @@ -136,3 +136,14 @@ right: var(--base-size-4); top: var(--base-size-4); } + +.IssueLink { + display: block; + border: var(--borderWidth-thin) solid var(--borderColor-default); + padding: var(--base-size-8); + transition: background 0.2s; +} + +.IssueLink:hover { + background-color: var(--bgColor-muted); +} diff --git a/packages/react/src/Overlay/Overlay.features.stories.tsx b/packages/react/src/Overlay/Overlay.features.stories.tsx index 4b89797ea61..925d6659e57 100644 --- a/packages/react/src/Overlay/Overlay.features.stories.tsx +++ b/packages/react/src/Overlay/Overlay.features.stories.tsx @@ -429,15 +429,7 @@ export const MemexIssueOverlay = ({role, open}: Args) => { event.preventDefault() setOverlayOpen(true) }} - sx={{ - display: 'block', - border: '1px solid', - borderColor: 'border.default', - p: 2, - ':hover': { - backgroundColor: 'canvas.subtle', - }, - }} + className={classes.IssueLink} > {title} diff --git a/packages/react/src/Popover/Popover.figma.tsx b/packages/react/src/Popover/Popover.figma.tsx index 4eb8d307373..5fae163a466 100644 --- a/packages/react/src/Popover/Popover.figma.tsx +++ b/packages/react/src/Popover/Popover.figma.tsx @@ -24,13 +24,7 @@ figma.connect(Popover, 'https://www.figma.com/design/GCvY3Qv8czRgZgvl1dG6lp/Prim example: ({caret, heading, body, action}) => ( - - {heading} - + {heading} {body} {action} diff --git a/packages/react/src/live-region/Announce.tsx b/packages/react/src/live-region/Announce.tsx index 15d697bb690..6e439d10a9b 100644 --- a/packages/react/src/live-region/Announce.tsx +++ b/packages/react/src/live-region/Announce.tsx @@ -3,11 +3,11 @@ import type React from 'react' import {useEffect, useRef, useState, type ElementRef} from 'react' import {useEffectOnce} from '../internal/hooks/useEffectOnce' import {useEffectCallback} from '../internal/hooks/useEffectCallback' -import type {PolymorphicProps} from '../utils/polymorphic2' +import type {PolymorphicProps} from '../utils/modern-polymorphic' export type AnnounceProps = PolymorphicProps< - 'div', As, + 'div', { /** * Specify if the content of the element should be announced when this diff --git a/packages/react/src/live-region/AriaAlert.tsx b/packages/react/src/live-region/AriaAlert.tsx index 7f9e48fa47f..78286a06abc 100644 --- a/packages/react/src/live-region/AriaAlert.tsx +++ b/packages/react/src/live-region/AriaAlert.tsx @@ -1,11 +1,11 @@ import type React from 'react' import {type ElementType} from 'react' import {Announce} from './Announce' -import type {PolymorphicProps} from '../utils/polymorphic2' +import type {PolymorphicProps} from '../utils/modern-polymorphic' export type AriaAlertProps = PolymorphicProps< - 'div', As, + 'div', { /** * Specify if the content of the element should be announced when this @@ -22,14 +22,6 @@ export type AriaAlertProps = PolymorphicProps< } > -export function AriaAlert({ - announceOnShow = true, - children, - ...rest -}: AriaAlertProps) { - return ( - - {children} - - ) +export function AriaAlert(props: AriaAlertProps) { + return } diff --git a/packages/react/src/live-region/AriaStatus.tsx b/packages/react/src/live-region/AriaStatus.tsx index a9a4ff13aac..6d237bc3445 100644 --- a/packages/react/src/live-region/AriaStatus.tsx +++ b/packages/react/src/live-region/AriaStatus.tsx @@ -1,11 +1,11 @@ import type React from 'react' import {type ElementType} from 'react' import {Announce} from './Announce' -import type {PolymorphicProps} from '../utils/polymorphic2' +import type {PolymorphicProps} from '../utils/modern-polymorphic' -export type AriaStatusProps = PolymorphicProps< - 'div', +export type AriaStatusProps = PolymorphicProps< As, + 'div', { /** * Specify if the content of the element should be announced when this @@ -27,14 +27,6 @@ export type AriaStatusProps = PolymorphicProps< } > -export function AriaStatus({ - announceOnShow = false, - children, - ...rest -}: AriaStatusProps) { - return ( - - {children} - - ) +export function AriaStatus(props: AriaStatusProps) { + return } diff --git a/packages/react/src/utils/modern-polymorphic.ts b/packages/react/src/utils/modern-polymorphic.ts new file mode 100644 index 00000000000..5f83844ac10 --- /dev/null +++ b/packages/react/src/utils/modern-polymorphic.ts @@ -0,0 +1,35 @@ +// Mostly taken from https://github.com/total-typescript/react-typescript-tutorial/blob/main/src/08-advanced-patterns/72-as-prop-with-forward-ref.solution.tsx + +import {forwardRef} from 'react' +import type {ComponentPropsWithRef, ElementType} from 'react' + +/** + * Distributive Omit utility type that works correctly with union types + */ +type DistributiveOmit = T extends unknown ? Omit : never + +/** + * Fixed version of forwardRef that provides better type inference for polymorphic components + */ +// TODO: figure out how to change this type so we can set displayName +// like this: `ComponentName.displayName = 'DisplayName' instead of using workarounds +type FixedForwardRef = = Record>( + render: (props: P, ref: React.Ref) => React.ReactNode, +) => (props: P & React.RefAttributes) => React.ReactNode + +/** + * Cast forwardRef to the fixed version with better type inference + */ +export const fixedForwardRef = forwardRef as FixedForwardRef + +/** + * Simplified polymorphic props type that handles the common pattern of + * `DistributiveOmit, 'as'>` + */ +export type PolymorphicProps< + TAs extends ElementType, + TDefaultElement extends ElementType = 'div', + Props extends Record = Record, +> = DistributiveOmit & Props, 'as'> & { + as?: TAs +} diff --git a/packages/react/src/utils/polymorphic2.ts b/packages/react/src/utils/polymorphic2.ts deleted file mode 100644 index b48264baa27..00000000000 --- a/packages/react/src/utils/polymorphic2.ts +++ /dev/null @@ -1,23 +0,0 @@ -/** - * This file is an alternative to polymorphic.ts that hopes to support - * polyrmophic components in React. It explicitly hopes to make it easy to - * type the props of components and allow for explicitly setting the type of a - * component - */ - -import type {ElementType} from 'react' - -type AsProp = { - /** - * Customize the element type of the container element for the component - */ - as?: As -} - -type PolymorphicProps = Props & - AsProp & - (As extends React.ElementType - ? Omit, keyof Props> - : Omit, keyof Props>) - -export type {PolymorphicProps} diff --git a/packages/styled-react/src/components/Link.tsx b/packages/styled-react/src/components/Link.tsx new file mode 100644 index 00000000000..c87a2629684 --- /dev/null +++ b/packages/styled-react/src/components/Link.tsx @@ -0,0 +1,12 @@ +import {Link as PrimerLink, type LinkProps as PrimerLinkProps} from '@primer/react' +import styled from 'styled-components' +import {sx, type SxProp} from '../sx' + +type LinkProps = PrimerLinkProps & SxProp + +const Link = styled(PrimerLink).withConfig({ + shouldForwardProp: prop => prop !== 'sx', +})` + ${sx} +` +export {Link, type LinkProps} diff --git a/packages/styled-react/src/index.tsx b/packages/styled-react/src/index.tsx index 8be52072298..d9064e9d81b 100644 --- a/packages/styled-react/src/index.tsx +++ b/packages/styled-react/src/index.tsx @@ -13,7 +13,6 @@ export {FormControl} from '@primer/react' export {Heading} from '@primer/react' export {IconButton} from '@primer/react' export {Label} from '@primer/react' -export {Link} from '@primer/react' export {NavList} from '@primer/react' export {Overlay} from '@primer/react' export {PageLayout} from '@primer/react' @@ -39,6 +38,7 @@ export {Checkbox, type CheckboxProps} from './components/Checkbox' export {CounterLabel, type CounterLabelProps} from './components/CounterLabel' export {Flash} from './components/Flash' export {Header, type HeaderProps} from './components/Header' +export {Link, type LinkProps} from './components/Link' export {LinkButton, type LinkButtonProps} from './components/LinkButton' export { PageHeader,