Update `Icon` and `Button` for SLDS2 #486

msmx-mnakagawa · 2025-06-06T01:01:27Z

What I did

Icon
- update icon types
- update markups and classnames
- update an interface about container
- improve a category icon classname
- add title, currentColor, and flip props
Button
- update markups and classnames
- add new button types
- reflect iconSize to ButtonIcon
- remove unused classnames

References

reg-suit · 2025-06-06T01:03:13Z

reg-suit detected visual differences.

Check this report, and review them.

⚪

🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵

What do the circles mean?

The number of circles represent the number of changed images.
🔴 : Changed items, ⚪ : New items, ⚫ : Deleted items, and 🔵 Passed items

How can I change the check status?

If reviewers approve this PR, the reg context status will be green automatically.

stomita

The shared_folder icon is not shown.

stomita · 2025-07-02T06:49:23Z

src/scripts/Icon.tsx

  }
 );

+export const SvgButtonIcon = (


Is there any special reason why SvgIcon can't be reused for button icons? Does it require a different treatment?

@stomita
Yes, there is.
SvgButtonIcon differs from SvgIcon in its logic for generating classnames, and, above all, in the context where it is used.

When I decided to define SvgButtonIcon separately, I judged it was too much to treat it as DRY.

However, it's also acceptable to unify them into a single component, so I did it in 288d955.

stomita · 2025-07-02T06:51:04Z

src/scripts/Icon.tsx

+  const { assetRoot = getAssetRoot() } = useContext(ComponentSettingsContext);
+
+  // icon and category prop should not include chars other than alphanumerics, underscore, and hyphen
+  const icon = (icon_ ?? '').replace(/[^\w-]/g, ''); // eslint-disable-line no-param-reassign


eslint-disable- comment might not be needed (as the line now using new const for assignment)

@stomita
I got it.
I've removed such comments in 1d538cc.

In addition, I've removed similar comments in other components as well in #490.

stomita · 2025-07-02T06:51:39Z

src/scripts/Icon.tsx

-          iconColor: fillIconColor,
-        }}
-      />
+    const ccontainerClassName = classnames(


containerClassName

@stomita
Well..., I just followed the original naming convention in the current master branch though...
However, I updated it in 9ea3e18.

stomita

Several icons in utility are not shown. If the SLDS ver in story is old, it must be synced with the lib update and adding new icon support.

This reverts commit fdc4e0d.

msmx-mnakagawa · 2025-07-03T02:28:57Z

@stomita

If the SLDS ver in story is old, it must be synced with the lib update and adding new icon support.

I got it.
I moved the commit fdc4e0d, which updates icon types, to #484.

stomita

Shows differently in VRT for new_custom action icons

msmx-mnakagawa · 2025-07-03T04:17:12Z

@stomita
Thank you for noticing that..

I removed the VRT unexpected diff in 5a141a3.

stomita · 2025-07-04T09:13:28Z

src/scripts/Icon.tsx

  size?: IconSize;
  align?: 'left' | 'right';
-  container?: IconContainer;
+  circleContainer?: boolean;


Since the container variance may not be limited to just circle, it's better to keep it as the current container property rather than making it a boolean.

@stomita
As far as confirming CSS classes in the spec, for now, only the variant of the container would be just a .slds-icon_container_circle.
https://v1.lightningdesignsystem.com/components/icons/#CSS-Class-Overview

Thus, I think currently it's enough to define this prop as a boolean.
However, at the same time, I understand your opinion, if we consider changes in the future.

Therefore, I've tried to revert it to the container prop in da113a2.

stomita · 2025-07-04T09:34:19Z

src/scripts/Icon.tsx

  tabIndex?: number;
  fillColor?: string;
+  title?: string;
+  currentColor?: boolean;


As the textColor and currentColor props are intended to be mutually exclusive and will not be used simultaneously, setting the textColor prop to "currentColor" can accurately reflect the intent of developers.

@stomita
As you pointed, indeed, the spec says that the currentColor and the other color settings (default, success etc) are mutually exclusive.
https://v1.lightningdesignsystem.com/components/icons/#Color

Therefore, I've tried including currentColor as one of variants of the textColor prop in 8235e6f.
Additionally, I added a story that we can confirm the currentColor behavior in 9bef3e3.

Do you think this solution would be fine?

stomita · 2025-07-04T12:37:20Z

src/scripts/Icon.tsx

  align?: 'left' | 'right';
-  container?: IconContainer;
+  circleContainer?: boolean;
  color?: string;


Maybe color is not used, right ?

@stomita
The color prop is used as rprops.
This usage is as the same as before this PR.

react-lightning-design-system/src/scripts/Icon.tsx

Lines 226 to 234 in 61e9ded

<svg

ref={ref}

className={iconClassNames}

aria-hidden

style={style}

{...rprops}

>

<use xlinkHref={iconUrl} />

</svg>

Although we can remove the color prop because it's also declared in SVGAttributes<SVGElement>, I decided to keep it on purpose because I thought it was more explicitly declared as documentation.

react-lightning-design-system/src/scripts/Icon.tsx

Lines 164 to 178 in 61e9ded

export type IconProps = {

label?: string;

containerClassName?: string;

category?: IconCategory;

icon: string;

size?: IconSize;

align?: 'left' | 'right';

container?: IconContainer;

color?: string;

textColor?: IconTextColor;

tabIndex?: number;

fillColor?: string;

title?: string;

flip?: boolean;

} & SVGAttributes<SVGElement>;

Which do you think is better: to keep it or to remove it?

If it is included in SVGAttributes<SVGElement> type and just passing to <svg>, it might not be necessary in the props definition.

@stomita
I got it. 👍
I removed duplicated declarations in 893d631.

stomita · 2025-07-04T12:46:42Z

src/scripts/Icon.tsx

  (props) => {
-    const { container, containerClassName, fillColor, ...rprops } = props;
+    const {
+      label,


If the label prop is only used for assistive text, it may be misleading, as label typically refers to something visible to users in other components. It might be better to share it with the title prop, unless there's a strong need to differentiate between the title and the assistive text.

@stomita
I can agree with your opinion.
Therefore, I unified the label prop to the title prop in f57f21b.

msmx-mnakagawa self-assigned this Jun 6, 2025

msmx-mnakagawa mentioned this pull request Jun 6, 2025

Prepare for SLDS2 #485

Merged

44 tasks

msmx-mnakagawa force-pushed the support-slds-2-icon-button branch 3 times, most recently from 82e31be to 4c04ff7 Compare June 20, 2025 02:12

msmx-mnakagawa force-pushed the support-slds-2-icon-button branch 2 times, most recently from ef12184 to ea7c2fa Compare July 1, 2025 07:52

msmx-mnakagawa added 5 commits July 1, 2025 18:31

(Icon): update icon types

fdc4e0d

(Icon): update markups and classnames

189b004

(Button): update markups and classnames

b05c36d

(Button): add new button types

df1c712

(Button): reflect iconSize to ButtonIcon

4e27579

msmx-mnakagawa force-pushed the support-slds-2-icon-button branch from ea7c2fa to 7041867 Compare July 1, 2025 09:38

msmx-mnakagawa added 4 commits July 1, 2025 18:48

(Button): remove unused classnames

096208e

(Icon): update an interface about container

90902c8

(Icon): improve a category icon classname

b78c6a6

(Icon): add title, currentColor, and flip props

73109a9

msmx-mnakagawa force-pushed the support-slds-2-icon-button branch from 7041867 to 73109a9 Compare July 1, 2025 09:49

msmx-mnakagawa requested a review from stomita July 1, 2025 09:55

msmx-mnakagawa marked this pull request as ready for review July 1, 2025 09:55

stomita reviewed Jul 2, 2025

View reviewed changes

stomita requested changes Jul 2, 2025

View reviewed changes

msmx-mnakagawa added 4 commits July 2, 2025 16:11

(Icon, Button): remove unnecessary eslint-disable- comments

1d538cc

(Icon): update a naming convention

9ea3e18

Revert "(Icon): update icon types"

87ab6e1

This reverts commit fdc4e0d.

(Icon, Button): unify SvgButtonIcon with SvgIcon

288d955

msmx-mnakagawa mentioned this pull request Jul 3, 2025

Remove unnecessary eslint-disable- comments for SLDS2 #490

Merged

msmx-mnakagawa requested a review from stomita July 3, 2025 02:45

stomita requested changes Jul 3, 2025

View reviewed changes

msmx-mnakagawa added 2 commits July 3, 2025 12:57

(Icon): reflect interface update to stories

ef99975

Merge branch 'support-slds-2' into support-slds-2-icon-button

5a141a3

msmx-mnakagawa force-pushed the support-slds-2-icon-button branch from 9b0c467 to 5a141a3 Compare July 3, 2025 03:57

msmx-mnakagawa requested a review from stomita July 3, 2025 04:17

stomita requested changes Jul 4, 2025

View reviewed changes

msmx-mnakagawa added 5 commits July 7, 2025 10:21

(Button): change the order of properties

49a0628

(Icon): replace circleContainer with container

da113a2

(Icon): make textColor and currentColor mutually exclusive

8235e6f

(Icon): add the Current Color story

9bef3e3

(Icon): unify the label into the title

f57f21b

msmx-mnakagawa force-pushed the support-slds-2-icon-button branch from 61e9ded to f57f21b Compare July 7, 2025 02:40

msmx-mnakagawa requested a review from stomita July 7, 2025 02:45

remove duplicated type definitions

893d631

stomita approved these changes Jul 10, 2025

View reviewed changes

stomita merged commit bf49d8d into support-slds-2 Jul 10, 2025
2 checks passed

	<svg
	ref={ref}
	className={iconClassNames}
	aria-hidden
	style={style}
	{...rprops}
	>
	<use xlinkHref={iconUrl} />
	</svg>

	export type IconProps = {
	label?: string;
	containerClassName?: string;
	category?: IconCategory;
	icon: string;
	size?: IconSize;
	align?: 'left' \| 'right';
	container?: IconContainer;
	color?: string;
	textColor?: IconTextColor;
	tabIndex?: number;
	fillColor?: string;
	title?: string;
	flip?: boolean;
	} & SVGAttributes<SVGElement>;

Update Icon and Button for SLDS2 #486

Update Icon and Button for SLDS2 #486

Conversation

msmx-mnakagawa commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What I did

References

Uh oh!

reg-suit bot commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stomita left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stomita left a comment

Choose a reason for hiding this comment

Uh oh!

msmx-mnakagawa commented Jul 3, 2025

Uh oh!

stomita left a comment

Choose a reason for hiding this comment

Uh oh!

msmx-mnakagawa commented Jul 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msmx-mnakagawa Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msmx-mnakagawa Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Update `Icon` and `Button` for SLDS2 #486

Update `Icon` and `Button` for SLDS2 #486

msmx-mnakagawa commented Jun 6, 2025 •

edited

Loading

reg-suit bot commented Jun 6, 2025 •

edited

Loading

msmx-mnakagawa Jul 7, 2025 •

edited

Loading

msmx-mnakagawa Jul 7, 2025 •

edited

Loading