From a9603b2304d462213c37f968f1a13f2a81cb535e Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Fri, 30 May 2025 09:50:34 +1000 Subject: [PATCH 1/4] fix: Icon styling rules for color --- packages/@react-spectrum/s2/src/Button.tsx | 11 +++++++---- .../@react-spectrum/s2/stories/Button.stories.tsx | 3 ++- packages/@react-spectrum/s2/style/index.ts | 4 ++-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/@react-spectrum/s2/src/Button.tsx b/packages/@react-spectrum/s2/src/Button.tsx index 0e25f55adc0..66eadf2b936 100644 --- a/packages/@react-spectrum/s2/src/Button.tsx +++ b/packages/@react-spectrum/s2/src/Button.tsx @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {baseColor, focusRing, fontRelative, lightDark, linearGradient, style} from '../style' with {type: 'macro'}; +import {baseColor, focusRing, fontRelative, iconStyle, lightDark, linearGradient, style} from '../style' with {type: 'macro'}; import {ButtonRenderProps, ContextValue, Link, LinkProps, OverlayTriggerStateContext, Provider, Button as RACButton, ButtonProps as RACButtonProps} from 'react-aria-components'; import {centerBaseline} from './CenterBaseline'; import {control, getAllowedOverrides, staticColor, StyleProps} from './style-utils' with {type: 'macro'}; @@ -27,6 +27,7 @@ import {useFocusableRef} from '@react-spectrum/utils'; import {useFormProps} from './Form'; import {useLocalizedStringFormatter} from '@react-aria/i18n'; import {useSpectrumContextProps} from './useSpectrumContextProps'; +import {mergeStyles} from '../style/runtime'; interface ButtonStyleProps { /** @@ -347,7 +348,7 @@ export const Button = forwardRef(function Button(props: ButtonProps, ref: Focusa isStaticColor: !!staticColor }, props.styles)}> {(renderProps) => (<> - {variant === 'genai' || variant === 'premium' + {variant === 'genai' || variant === 'premium' ? ( {typeof props.children === 'string' ? {props.children} : props.children} diff --git a/packages/@react-spectrum/s2/stories/Button.stories.tsx b/packages/@react-spectrum/s2/stories/Button.stories.tsx index df3c26a4a28..9f2996f9bdb 100644 --- a/packages/@react-spectrum/s2/stories/Button.stories.tsx +++ b/packages/@react-spectrum/s2/stories/Button.stories.tsx @@ -16,6 +16,7 @@ import {categorizeArgTypes, StaticColorDecorator} from './utils'; import type {Meta, StoryObj} from '@storybook/react'; import NewIcon from '../s2wf-icons/S2_Icon_New_20_N.svg'; import {style} from '../style/spectrum-theme' with { type: 'macro' }; +import {iconStyle} from '../style' with { type: 'macro' }; import {useEffect, useRef, useState} from 'react'; const meta: Meta = { @@ -39,7 +40,7 @@ export const Example: Story = { return (
- + diff --git a/packages/@react-spectrum/s2/style/index.ts b/packages/@react-spectrum/s2/style/index.ts index fb2c934e69a..1f8090a809b 100644 --- a/packages/@react-spectrum/s2/style/index.ts +++ b/packages/@react-spectrum/s2/style/index.ts @@ -43,7 +43,7 @@ export const focusRing = () => ({ interface IconStyle { size?: 'XS' | 'S' | 'M' | 'L' |'XL', - color?: 'white' | 'black' | 'accent' | 'neutral' | 'negative' | 'informative' | 'positive' | 'notice' | 'gray' | 'red' | 'orange' | 'yellow' | 'chartreuse' | 'celery' | 'green' | 'seafoam' | 'cyan' | 'blue' | 'indigo' | 'purple' | 'fuchsia' | 'magenta' | 'pink' | 'turquoise' | 'cinnamon' | 'brown' | 'silver', + color?: 'white' | 'black' | 'accent' | 'neutral' | 'negative' | 'informative' | 'positive' | 'notice' | 'gray' | 'red' | 'orange' | 'yellow' | 'chartreuse' | 'celery' | 'green' | 'seafoam' | 'cyan' | 'blue' | 'indigo' | 'purple' | 'fuchsia' | 'magenta' | 'pink' | 'turquoise' | 'cinnamon' | 'brown' | 'silver' | 'inherit', margin?: Spacing, marginStart?: Spacing, marginEnd?: Spacing, @@ -81,7 +81,7 @@ const iconSizes = { export function iconStyle(this: MacroContext | void, options: IconStyle): StyleString> { let {size = 'M', color, ...styles} = options; - + if (color) { styles['--iconPrimary'] = { type: 'fill', From 53bd465a73a9e607e52ef633c941b81a77e9f67d Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Fri, 30 May 2025 09:59:36 +1000 Subject: [PATCH 2/4] make currentColor --- packages/@react-spectrum/s2/src/Button.tsx | 6 +----- packages/@react-spectrum/s2/style/index.ts | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/@react-spectrum/s2/src/Button.tsx b/packages/@react-spectrum/s2/src/Button.tsx index 66eadf2b936..2a95fe09256 100644 --- a/packages/@react-spectrum/s2/src/Button.tsx +++ b/packages/@react-spectrum/s2/src/Button.tsx @@ -244,10 +244,6 @@ const button = style ({ interface IconStyle { size?: 'XS' | 'S' | 'M' | 'L' |'XL', - color?: 'white' | 'black' | 'accent' | 'neutral' | 'negative' | 'informative' | 'positive' | 'notice' | 'gray' | 'red' | 'orange' | 'yellow' | 'chartreuse' | 'celery' | 'green' | 'seafoam' | 'cyan' | 'blue' | 'indigo' | 'purple' | 'fuchsia' | 'magenta' | 'pink' | 'turquoise' | 'cinnamon' | 'brown' | 'silver' | 'inherit', + color?: 'white' | 'black' | 'accent' | 'neutral' | 'negative' | 'informative' | 'positive' | 'notice' | 'gray' | 'red' | 'orange' | 'yellow' | 'chartreuse' | 'celery' | 'green' | 'seafoam' | 'cyan' | 'blue' | 'indigo' | 'purple' | 'fuchsia' | 'magenta' | 'pink' | 'turquoise' | 'cinnamon' | 'brown' | 'silver' | 'currentColor', margin?: Spacing, marginStart?: Spacing, marginEnd?: Spacing, From 6d49d030e5c78080ab54dd6e7b6b92aaab0de45c Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Fri, 30 May 2025 13:59:51 +1000 Subject: [PATCH 3/4] fix other usages --- .../@react-spectrum/s2/src/ActionButton.tsx | 12 +++++----- packages/@react-spectrum/s2/src/Badge.tsx | 9 +++---- packages/@react-spectrum/s2/src/Button.tsx | 24 +++++++++---------- packages/@react-spectrum/s2/src/ComboBox.tsx | 5 ++-- packages/@react-spectrum/s2/src/Menu.tsx | 18 ++++---------- packages/@react-spectrum/s2/src/Picker.tsx | 7 +++--- .../s2/src/SegmentedControl.tsx | 9 +++---- packages/@react-spectrum/s2/src/Tabs.tsx | 11 ++++----- .../@react-spectrum/s2/src/TabsPicker.tsx | 5 ++-- packages/@react-spectrum/s2/src/TagGroup.tsx | 9 +++---- packages/@react-spectrum/s2/src/TreeView.tsx | 11 ++++----- .../s2/stories/Button.stories.tsx | 3 +-- 12 files changed, 51 insertions(+), 72 deletions(-) diff --git a/packages/@react-spectrum/s2/src/ActionButton.tsx b/packages/@react-spectrum/s2/src/ActionButton.tsx index 1f1dd8ee056..cc4052693e5 100644 --- a/packages/@react-spectrum/s2/src/ActionButton.tsx +++ b/packages/@react-spectrum/s2/src/ActionButton.tsx @@ -12,13 +12,14 @@ import {ActionButtonGroupContext} from './ActionButtonGroup'; import {AvatarContext} from './Avatar'; -import {baseColor, focusRing, fontRelative, lightDark, style} from '../style' with { type: 'macro' }; +import {baseColor, focusRing, fontRelative, iconStyle, lightDark, style} from '../style' with { type: 'macro' }; import {ButtonProps, ButtonRenderProps, ContextValue, OverlayTriggerStateContext, Provider, Button as RACButton, useSlottedContext} from 'react-aria-components'; import {centerBaseline} from './CenterBaseline'; import {control, getAllowedOverrides, staticColor, StyleProps} from './style-utils' with { type: 'macro' }; import {createContext, forwardRef, ReactNode, useContext} from 'react'; import {FocusableRef, FocusableRefValue} from '@react-types/shared'; import {IconContext} from './Icon'; +import {mergeStyles} from '../style/runtime'; import {NotificationBadgeContext} from './NotificationBadge'; import {pressScale} from './pressScale'; import {SkeletonContext} from './Skeleton'; @@ -144,10 +145,6 @@ export const btnStyles = style({ } } } - }, - '--iconPrimary': { - type: 'fill', - value: 'currentColor' } }, getAllowedOverrides()); @@ -194,7 +191,7 @@ export const Badge = forwardRef(function Badge(props: BadgeProps, ref: DOMRef diff --git a/packages/@react-spectrum/s2/src/Button.tsx b/packages/@react-spectrum/s2/src/Button.tsx index 2a95fe09256..c018809d318 100644 --- a/packages/@react-spectrum/s2/src/Button.tsx +++ b/packages/@react-spectrum/s2/src/Button.tsx @@ -19,6 +19,7 @@ import {FocusableRef, FocusableRefValue} from '@react-types/shared'; import {IconContext} from './Icon'; // @ts-ignore import intlMessages from '../intl/*.json'; +import {mergeStyles} from '../style/runtime'; import {pressScale} from './pressScale'; import {ProgressCircle} from './ProgressCircle'; import {SkeletonContext} from './Skeleton'; @@ -27,7 +28,6 @@ import {useFocusableRef} from '@react-spectrum/utils'; import {useFormProps} from './Form'; import {useLocalizedStringFormatter} from '@react-aria/i18n'; import {useSpectrumContextProps} from './useSpectrumContextProps'; -import {mergeStyles} from '../style/runtime'; interface ButtonStyleProps { /** @@ -373,17 +373,17 @@ export const Button = forwardRef(function Button(props: ButtonProps, ref: Focusa }], [IconContext, { render: centerBaseline({slot: 'icon', styles: style({order: 0})}), - styles: mergeStyles(iconStyle({ - color: 'currentColor' - }), style({ - size: fontRelative(20), - marginStart: '--iconMargin', - flexShrink: 0, - opacity: { - default: 1, - isProgressVisible: 0 - } - })({isProgressVisible})) + styles: mergeStyles( + iconStyle({color: 'currentColor'}), + style({ + size: fontRelative(20), + marginStart: '--iconMargin', + flexShrink: 0, + opacity: { + default: 1, + isProgressVisible: 0 + } + })({isProgressVisible})) }] ]}> {typeof props.children === 'string' ? {props.children} : props.children} diff --git a/packages/@react-spectrum/s2/src/ComboBox.tsx b/packages/@react-spectrum/s2/src/ComboBox.tsx index dfa9c9c7274..34a4c69ddfc 100644 --- a/packages/@react-spectrum/s2/src/ComboBox.tsx +++ b/packages/@react-spectrum/s2/src/ComboBox.tsx @@ -33,7 +33,7 @@ import { Virtualizer } from 'react-aria-components'; import {AsyncLoadable, HelpTextProps, LoadingState, SpectrumLabelableProps} from '@react-types/shared'; -import {baseColor, edgeToText, focusRing, space, style} from '../style' with {type: 'macro'}; +import {baseColor, edgeToText, focusRing, iconStyle, space, style} from '../style' with {type: 'macro'}; import {centerBaseline} from './CenterBaseline'; import {centerPadding, control, controlBorderRadius, controlFont, controlSize, field, fieldInput, getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro'}; import { @@ -57,6 +57,7 @@ import {IconContext} from './Icon'; // @ts-ignore import intlMessages from '../intl/*.json'; import {mergeRefs, useResizeObserver, useSlotId} from '@react-aria/utils'; +import {mergeStyles} from '../style/runtime'; import {Node} from 'react-stately'; import {Placement} from 'react-aria'; import {PopoverBase} from './Popover'; @@ -389,7 +390,7 @@ export function ComboBoxItem(props: ComboBoxItemProps): ReactNode { values={[ [IconContext, { slots: { - icon: {render: centerBaseline({slot: 'icon', styles: iconCenterWrapper}), styles: icon} + icon: {render: centerBaseline({slot: 'icon', styles: iconCenterWrapper}), styles: mergeStyles(iconStyle({color: 'currentColor'}), icon)} } }], [TextContext, { diff --git a/packages/@react-spectrum/s2/src/Menu.tsx b/packages/@react-spectrum/s2/src/Menu.tsx index aa990d5744b..783516eb964 100644 --- a/packages/@react-spectrum/s2/src/Menu.tsx +++ b/packages/@react-spectrum/s2/src/Menu.tsx @@ -28,7 +28,7 @@ import { Separator, SeparatorProps } from 'react-aria-components'; -import {baseColor, edgeToText, focusRing, fontRelative, size, space, style} from '../style' with {type: 'macro'}; +import {baseColor, edgeToText, focusRing, fontRelative, iconStyle, size, space, style} from '../style' with {type: 'macro'}; import {box, iconStyles} from './Checkbox'; import {centerBaseline} from './CenterBaseline'; import {centerPadding, control, controlFont, controlSize, getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro'}; @@ -221,11 +221,7 @@ export let icon = style({ size: fontRelative(20), // too small default icon size is wrong, it's like the icons are 1 tshirt size bigger than the rest of the component? check again after typography changes // reminder, size of WF is applied via font size - marginEnd: 'text-to-visual', - '--iconPrimary': { - type: 'fill', - value: 'currentColor' - } + marginEnd: 'text-to-visual' }); export let iconCenterWrapper = style({ @@ -305,11 +301,7 @@ let keyboard = style<{size: 'S' | 'M' | 'L' | 'XL', isDisabled: boolean}>({ let descriptor = style({ gridArea: 'descriptor', - marginStart: 8, - '--iconPrimary': { - type: 'fill', - value: 'currentColor' - } + marginStart: 8 }); let InternalMenuContext = createContext<{size: 'S' | 'M' | 'L' | 'XL', isSubmenu: boolean, hideLinkOutIcon: boolean}>({ @@ -486,8 +478,8 @@ export function MenuItem(props: MenuItemProps): ReactNode { values={[ [IconContext, { slots: { - icon: {render: centerBaseline({slot: 'icon', styles: iconCenterWrapper}), styles: icon}, - descriptor: {render: centerBaseline({slot: 'descriptor', styles: descriptor})} // TODO: remove once we have default? + icon: {render: centerBaseline({slot: 'icon', styles: iconCenterWrapper}), styles: mergeStyles(iconStyle({color: 'currentColor'}), icon)}, + descriptor: {render: centerBaseline({slot: 'descriptor', styles: descriptor}), styles: iconStyle({color: 'currentColor'})} // TODO: remove once we have default? } }], [TextContext, { diff --git a/packages/@react-spectrum/s2/src/Picker.tsx b/packages/@react-spectrum/s2/src/Picker.tsx index 1a0e885444c..de22b1c09ba 100644 --- a/packages/@react-spectrum/s2/src/Picker.tsx +++ b/packages/@react-spectrum/s2/src/Picker.tsx @@ -32,7 +32,7 @@ import { Virtualizer } from 'react-aria-components'; import {AsyncLoadable, FocusableRef, FocusableRefValue, HelpTextProps, LoadingState, PressEvent, RefObject, SpectrumLabelableProps} from '@react-types/shared'; -import {baseColor, edgeToText, focusRing, style} from '../style' with {type: 'macro'}; +import {baseColor, edgeToText, focusRing, iconStyle, style} from '../style' with {type: 'macro'}; import {centerBaseline} from './CenterBaseline'; import { checkmark, @@ -64,6 +64,7 @@ import {HeaderContext, HeadingContext, Text, TextContext} from './Content'; import {IconContext} from './Icon'; // @ts-ignore import intlMessages from '../intl/*.json'; +import {mergeStyles} from '../style/runtime'; import {Placement} from 'react-aria'; import {PopoverBase} from './Popover'; import {PressResponder} from '@react-aria/interactions'; @@ -514,7 +515,7 @@ const PickerButton = createHideableComponent(function PickerButton ({ userSelect: 'none', backgroundColor: 'transparent', borderStyle: 'none', - '--iconPrimary': { - type: 'fill', - value: 'currentColor' - }, // The selected item has lower z-index so that the sliding background // animation does not cover other items. zIndex: { @@ -264,7 +260,8 @@ export const SegmentedControlItem = /*#__PURE__*/ forwardRef(function SegmentedC {typeof props.children === 'string' ? {props.children} : props.children} diff --git a/packages/@react-spectrum/s2/src/TabsPicker.tsx b/packages/@react-spectrum/s2/src/TabsPicker.tsx index a480bcfbca8..7a2dd7f1ff2 100644 --- a/packages/@react-spectrum/s2/src/TabsPicker.tsx +++ b/packages/@react-spectrum/s2/src/TabsPicker.tsx @@ -23,7 +23,7 @@ import { Provider, SelectValue } from 'react-aria-components'; -import {baseColor, edgeToText, focusRing, size, style} from '../style' with {type: 'macro'}; +import {baseColor, edgeToText, focusRing, iconStyle, size, style} from '../style' with {type: 'macro'}; import {centerBaseline} from './CenterBaseline'; import { checkmark, @@ -44,6 +44,7 @@ import {FocusableRef, FocusableRefValue, SpectrumLabelableProps} from '@react-ty import {forwardRefType} from './types'; import {HeaderContext, HeadingContext, Text, TextContext} from './Content'; import {IconContext} from './Icon'; +import {mergeStyles} from '../style/runtime'; import {Placement} from 'react-aria'; import {PopoverBase} from './Popover'; import {pressScale} from './pressScale'; @@ -207,7 +208,7 @@ function Picker(props: PickerProps, ref: FocusableRef = { @@ -40,7 +39,7 @@ export const Example: Story = { return (
- + From 8a010dc3631e4cea042c63994835fa2d87fff1c1 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Fri, 30 May 2025 15:30:54 +1000 Subject: [PATCH 4/4] fix chromatic catches --- packages/@react-spectrum/s2/src/Button.tsx | 5 ++++- packages/@react-spectrum/s2/src/Menu.tsx | 8 ++++++++ packages/@react-spectrum/s2/src/ToggleButton.tsx | 5 +++-- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/@react-spectrum/s2/src/Button.tsx b/packages/@react-spectrum/s2/src/Button.tsx index c018809d318..a222ddb8abf 100644 --- a/packages/@react-spectrum/s2/src/Button.tsx +++ b/packages/@react-spectrum/s2/src/Button.tsx @@ -457,7 +457,10 @@ export const LinkButton = forwardRef(function LinkButton(props: LinkButtonProps, }], [IconContext, { render: centerBaseline({slot: 'icon', styles: style({order: 0})}), - styles: style({size: fontRelative(20), marginStart: '--iconMargin', flexShrink: 0}) + styles: mergeStyles( + iconStyle({color: 'currentColor'}), + style({size: fontRelative(20), marginStart: '--iconMargin', flexShrink: 0}) + ) }] ]}> {typeof props.children === 'string' ? {props.children} : props.children} diff --git a/packages/@react-spectrum/s2/src/Menu.tsx b/packages/@react-spectrum/s2/src/Menu.tsx index 783516eb964..7ae1d136af1 100644 --- a/packages/@react-spectrum/s2/src/Menu.tsx +++ b/packages/@react-spectrum/s2/src/Menu.tsx @@ -509,6 +509,10 @@ export function MenuItem(props: MenuItemProps): ReactNode { direction: { rtl: -1 } + }, + '--iconPrimary': { + type: 'fill', + value: 'currentColor' } })({direction})} />
@@ -522,6 +526,10 @@ export function MenuItem(props: MenuItemProps): ReactNode { direction: { rtl: -1 } + }, + '--iconPrimary': { + type: 'fill', + value: 'currentColor' } })({direction})} />
diff --git a/packages/@react-spectrum/s2/src/ToggleButton.tsx b/packages/@react-spectrum/s2/src/ToggleButton.tsx index b5041a3289a..c5f608af59e 100644 --- a/packages/@react-spectrum/s2/src/ToggleButton.tsx +++ b/packages/@react-spectrum/s2/src/ToggleButton.tsx @@ -15,8 +15,9 @@ import {centerBaseline} from './CenterBaseline'; import {ContextValue, Provider, ToggleButton as RACToggleButton, ToggleButtonProps as RACToggleButtonProps, useSlottedContext} from 'react-aria-components'; import {createContext, forwardRef, ReactNode} from 'react'; import {FocusableRef, FocusableRefValue} from '@react-types/shared'; -import {fontRelative, style} from '../style' with {type: 'macro'}; +import {fontRelative, iconStyle, style} from '../style' with {type: 'macro'}; import {IconContext} from './Icon'; +import {mergeStyles} from '../style/runtime'; import {pressScale} from './pressScale'; import {SkeletonContext} from './Skeleton'; import {StyleProps} from './style-utils'; @@ -81,7 +82,7 @@ export const ToggleButton = forwardRef(function ToggleButton(props: ToggleButton [TextContext, {styles: style({paddingY: '--labelPadding', order: 1, truncate: true})}], [IconContext, { render: centerBaseline({slot: 'icon', styles: style({order: 0})}), - styles: style({size: fontRelative(20), marginStart: '--iconMargin', flexShrink: 0}) + styles: mergeStyles(iconStyle({color: 'currentColor'}), style({size: fontRelative(20), marginStart: '--iconMargin', flexShrink: 0})) }] ]}> {typeof props.children === 'string' ? {props.children} : props.children}