From 954abf33ad66b95165db79c836c424ebf6742d75 Mon Sep 17 00:00:00 2001 From: Cameron Dutro Date: Thu, 19 Sep 2024 15:01:44 -0700 Subject: [PATCH 1/5] Support passing React.ReactElements for icons --- packages/react/src/Autocomplete/AutocompleteMenu.tsx | 9 +++++---- .../src/SegmentedControl/SegmentedControlButton.tsx | 9 +++------ .../src/SegmentedControl/SegmentedControlIconButton.tsx | 7 +++---- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/packages/react/src/Autocomplete/AutocompleteMenu.tsx b/packages/react/src/Autocomplete/AutocompleteMenu.tsx index fa566f3bbaf..cb09cdacc2c 100644 --- a/packages/react/src/Autocomplete/AutocompleteMenu.tsx +++ b/packages/react/src/Autocomplete/AutocompleteMenu.tsx @@ -12,12 +12,13 @@ import {AutocompleteContext} from './AutocompleteContext' import type {IconProps} from '@primer/octicons-react' import {PlusIcon} from '@primer/octicons-react' import VisuallyHidden from '../_VisuallyHidden' +import {isElement} from 'react-is' type OnSelectedChange = (item: T | T[]) => void export type AutocompleteMenuItem = MandateProps & { - leadingVisual?: React.FunctionComponent> + leadingVisual?: React.FunctionComponent> | React.ReactElement text?: string - trailingVisual?: React.FunctionComponent> + trailingVisual?: React.FunctionComponent> | React.ReactElement } const getDefaultSortFn = (isItemSelectedFn: (itemId: string) => boolean) => (itemIdA: string, itemIdB: string) => @@ -352,13 +353,13 @@ function AutocompleteMenu(props: AutocompleteMe onAction(item)} {...itemProps} id={id} data-id={id}> {LeadingVisual && ( - + {isElement(LeadingVisual) ? LeadingVisual : } )} {children ?? text} {TrailingVisual && ( - + {isElement(TrailingVisual) ? TrailingVisual : } )} diff --git a/packages/react/src/SegmentedControl/SegmentedControlButton.tsx b/packages/react/src/SegmentedControl/SegmentedControlButton.tsx index 58003d167f5..95099d8830b 100644 --- a/packages/react/src/SegmentedControl/SegmentedControlButton.tsx +++ b/packages/react/src/SegmentedControl/SegmentedControlButton.tsx @@ -7,6 +7,7 @@ import type {SxProp} from '../sx' import sx, {merge} from '../sx' import {getSegmentedControlButtonStyles, getSegmentedControlListItemStyles} from './getSegmentedControlStyles' import {defaultSxProp} from '../utils/defaultSxProp' +import {isElement} from 'react-is' export type SegmentedControlButtonProps = { /** The visible label rendered in the button */ @@ -16,7 +17,7 @@ export type SegmentedControlButtonProps = { /** Whether the segment is selected. This is used for uncontrolled `SegmentedControls` to pick one `SegmentedControlButton` that is selected on the initial render. */ defaultSelected?: boolean /** The leading icon comes before item label */ - leadingIcon?: React.FunctionComponent> + leadingIcon?: React.FunctionComponent> | React.ReactElement } & SxProp & ButtonHTMLAttributes @@ -42,11 +43,7 @@ const SegmentedControlButton: React.FC - {LeadingIcon && ( - - - - )} + {LeadingIcon && {isElement(LeadingIcon) ? LeadingIcon : }} {children} diff --git a/packages/react/src/SegmentedControl/SegmentedControlIconButton.tsx b/packages/react/src/SegmentedControl/SegmentedControlIconButton.tsx index 11970c0ab47..4028260fc9f 100644 --- a/packages/react/src/SegmentedControl/SegmentedControlIconButton.tsx +++ b/packages/react/src/SegmentedControl/SegmentedControlIconButton.tsx @@ -7,11 +7,12 @@ import sx, {merge} from '../sx' import {getSegmentedControlButtonStyles, getSegmentedControlListItemStyles} from './getSegmentedControlStyles' import Box from '../Box' import {defaultSxProp} from '../utils/defaultSxProp' +import {isElement} from 'react-is' export type SegmentedControlIconButtonProps = { 'aria-label': string /** The icon that represents the segmented control item */ - icon: React.FunctionComponent> + icon: React.FunctionComponent> | React.ReactElement /** Whether the segment is selected. This is used for controlled SegmentedControls, and needs to be updated using the onChange handler on SegmentedControl. */ selected?: boolean /** Whether the segment is selected. This is used for uncontrolled SegmentedControls to pick one SegmentedControlButton that is selected on the initial render. */ @@ -53,9 +54,7 @@ export const SegmentedControlIconButton: React.FC - - - + {isElement(Icon) ? Icon : } ) From 4f2e202992d29f6a7f84dc41433c8eefeb439d70 Mon Sep 17 00:00:00 2001 From: Cameron Dutro Date: Thu, 19 Sep 2024 15:18:36 -0700 Subject: [PATCH 2/5] Add changeset --- .changeset/shiny-otters-call.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/shiny-otters-call.md diff --git a/.changeset/shiny-otters-call.md b/.changeset/shiny-otters-call.md new file mode 100644 index 00000000000..bc858921712 --- /dev/null +++ b/.changeset/shiny-otters-call.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +[SegmentedControl, Autocomplete] Support passing React.ReactElements for icons. From b431a8d7d8d85d09b9eaf49ed2d434378049e0d8 Mon Sep 17 00:00:00 2001 From: Cameron Dutro Date: Thu, 19 Sep 2024 15:46:14 -0700 Subject: [PATCH 3/5] Fix types --- packages/react/src/Button/ButtonBase.tsx | 22 ++++++++++++++---- packages/react/src/Button/types.ts | 7 +++--- .../src/SegmentedControl/SegmentedControl.tsx | 23 +++++++++++++++---- 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/packages/react/src/Button/ButtonBase.tsx b/packages/react/src/Button/ButtonBase.tsx index 8f1bc8b856c..e3ee6c99f6e 100644 --- a/packages/react/src/Button/ButtonBase.tsx +++ b/packages/react/src/Button/ButtonBase.tsx @@ -19,24 +19,30 @@ import {AriaStatus} from '../live-region' import {clsx} from 'clsx' import classes from './ButtonBase.module.css' import {useFeatureFlag} from '../FeatureFlags' +import {isElement} from 'react-is' const iconWrapStyles = { display: 'flex', pointerEvents: 'none', } -const renderVisual = (Visual: React.ElementType, loading: boolean, visualName: string) => ( +const renderVisual = (Visual: React.ElementType | React.ReactElement, loading: boolean, visualName: string) => ( - {loading ? : } + {loading ? : isElement(Visual) ? Visual : } ) -const renderModuleVisual = (Visual: React.ElementType, loading: boolean, visualName: string, counterLabel: boolean) => ( +const renderModuleVisual = ( + Visual: React.ElementType | React.ReactElement, + loading: boolean, + visualName: string, + counterLabel: boolean, +) => ( - {loading ? : } + {loading ? : isElement(Visual) ? Visual : } ) @@ -141,6 +147,8 @@ const ButtonBase = forwardRef( {Icon ? ( loading ? ( + ) : isElement(Icon) ? ( + Icon ) : ( ) @@ -163,7 +171,7 @@ const ButtonBase = forwardRef( } { /* Render a leading visual unless the button is in a loading state. - Then replace the leading visual with a loading spinner. */ + Then replace the leading visual with a loading spinner. */ LeadingVisual && renderModuleVisual(LeadingVisual, Boolean(loading), 'leadingVisual', false) } {children && ( @@ -260,6 +268,8 @@ const ButtonBase = forwardRef( {Icon ? ( loading ? ( + ) : isElement(Icon) ? ( + Icon ) : ( ) @@ -369,6 +379,8 @@ const ButtonBase = forwardRef( {Icon ? ( loading ? ( + ) : isElement(Icon) ? ( + Icon ) : ( ) diff --git a/packages/react/src/Button/types.ts b/packages/react/src/Button/types.ts index 0d5dfb08e43..2d3fae5b07b 100644 --- a/packages/react/src/Button/types.ts +++ b/packages/react/src/Button/types.ts @@ -4,6 +4,7 @@ import type {SxProp} from '../sx' import sx from '../sx' import getGlobalFocusStyles from '../internal/utils/getGlobalFocusStyles' import type {TooltipDirection} from '../TooltipV2' +import type {IconProps} from '@primer/octicons-react' export const StyledButton = styled.button` ${getGlobalFocusStyles('-2px')}; @@ -66,17 +67,17 @@ export type ButtonProps = { /** * The icon for the IconButton */ - icon?: React.ElementType | null + icon?: React.FunctionComponent | React.ElementType | React.ReactElement | null /** * The leading visual which comes before the button content */ - leadingVisual?: React.ElementType | null + leadingVisual?: React.ElementType | React.ReactElement | null /** * The trailing visual which comes after the button content */ - trailingVisual?: React.ElementType | null + trailingVisual?: React.ElementType | React.ReactElement | null /** * Trailing action appears to the right of the trailing visual and is always locked to the end diff --git a/packages/react/src/SegmentedControl/SegmentedControl.tsx b/packages/react/src/SegmentedControl/SegmentedControl.tsx index cf838c21b4a..41e81c792e2 100644 --- a/packages/react/src/SegmentedControl/SegmentedControl.tsx +++ b/packages/react/src/SegmentedControl/SegmentedControl.tsx @@ -13,6 +13,7 @@ import {useResponsiveValue} from '../hooks/useResponsiveValue' import type {WidthOnlyViewportRangeKeys} from '../utils/types/ViewportRangeKeys' import styled from 'styled-components' import {defaultSxProp} from '../utils/defaultSxProp' +import {isElement} from 'react-is' // Needed because passing a ref to `Box` causes a type error const SegmentedControlList = styled.ul` @@ -80,16 +81,30 @@ const Root: React.FC> = ({ ) ? React.Children.toArray(children)[selectedIndex] : undefined - const getChildIcon = (childArg: React.ReactNode) => { + const getChildIcon = (childArg: React.ReactNode): React.ReactElement | null => { if ( React.isValidElement(childArg) && childArg.type === Button && childArg.props.leadingIcon ) { - return childArg.props.leadingIcon + if (isElement(childArg.props.leadingIcon)) { + return childArg.props.leadingIcon + } else { + const LeadingIcon = childArg.props.leadingIcon + return + } } - return React.isValidElement(childArg) ? childArg.props.icon : null + if (React.isValidElement(childArg)) { + if (isElement(childArg.props.icon)) { + childArg.props.icon + } else { + const Icon = childArg.props.icon + return + } + } + + return null } const getChildText = (childArg: React.ReactNode) => { if (React.isValidElement(childArg) && childArg.type === Button) { @@ -140,7 +155,7 @@ const Root: React.FC> = ({ child.props.onClick && child.props.onClick(event as React.MouseEvent) }} > - {ChildIcon && } {getChildText(child)} + {ChildIcon} {getChildText(child)} ) })} From 6b9994467ed6e56b88dd4b47bc8d282a3fec784b Mon Sep 17 00:00:00 2001 From: Cameron Dutro Date: Thu, 19 Sep 2024 16:28:13 -0700 Subject: [PATCH 4/5] Woops, fix tests --- packages/react/src/SegmentedControl/SegmentedControl.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/react/src/SegmentedControl/SegmentedControl.tsx b/packages/react/src/SegmentedControl/SegmentedControl.tsx index 41e81c792e2..fe9190e35dd 100644 --- a/packages/react/src/SegmentedControl/SegmentedControl.tsx +++ b/packages/react/src/SegmentedControl/SegmentedControl.tsx @@ -95,7 +95,10 @@ const Root: React.FC> = ({ } } - if (React.isValidElement(childArg)) { + if ( + React.isValidElement(childArg) && + childArg.type === SegmentedControlIconButton + ) { if (isElement(childArg.props.icon)) { childArg.props.icon } else { From 8ce4104fff13ccff9927acbe84b4f72322f38883 Mon Sep 17 00:00:00 2001 From: Cameron Dutro Date: Tue, 8 Oct 2024 11:43:49 -0700 Subject: [PATCH 5/5] Add type tests --- .../Button/__tests__/Button.types.test.tsx | 10 +++++++- .../SegmentedControl.types.test.tsx | 21 ++++++++++++++++ .../src/__tests__/Autocomplete.types.test.tsx | 24 +++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 packages/react/src/SegmentedControl/SegmentedControl.types.test.tsx create mode 100644 packages/react/src/__tests__/Autocomplete.types.test.tsx diff --git a/packages/react/src/Button/__tests__/Button.types.test.tsx b/packages/react/src/Button/__tests__/Button.types.test.tsx index b50f0c51d24..23b0a084975 100644 --- a/packages/react/src/Button/__tests__/Button.types.test.tsx +++ b/packages/react/src/Button/__tests__/Button.types.test.tsx @@ -1,4 +1,4 @@ -import {StopIcon} from '@primer/octicons-react' +import {LogoGithubIcon, StopIcon} from '@primer/octicons-react' import React, {useRef} from 'react' import {Button, IconButton} from '../../Button' @@ -80,3 +80,11 @@ export function supportsLeadingVisual() { export function supportsTrailingVisual() { return } + +export function supportsLeadingVisualElement() { + return +} + +export function supportsTrailingVisualElement() { + return +} diff --git a/packages/react/src/SegmentedControl/SegmentedControl.types.test.tsx b/packages/react/src/SegmentedControl/SegmentedControl.types.test.tsx new file mode 100644 index 00000000000..2d0dd73a461 --- /dev/null +++ b/packages/react/src/SegmentedControl/SegmentedControl.types.test.tsx @@ -0,0 +1,21 @@ +import {LogoGithubIcon} from '@primer/octicons-react' +import {SegmentedControl} from './SegmentedControl' +import React from 'react' + +export function buttonWithLeadingIconElement() { + return ( + + }>Button + + ) +} + +export function iconButtonWithIconElement() { + return ( + + }> + Button + + + ) +} diff --git a/packages/react/src/__tests__/Autocomplete.types.test.tsx b/packages/react/src/__tests__/Autocomplete.types.test.tsx new file mode 100644 index 00000000000..185480459cd --- /dev/null +++ b/packages/react/src/__tests__/Autocomplete.types.test.tsx @@ -0,0 +1,24 @@ +import {LogoGithubIcon} from '@primer/octicons-react' +import {Autocomplete} from '..' +import React from 'react' + +export function itemWithIconElements() { + return ( + <> + + + + , trailingVisual: }, + ]} + > + + + + ) +}