Skip to content

Conversation

@kimlisa
Copy link
Contributor

@kimlisa kimlisa commented Dec 5, 2025

part of https://github.com/gravitational/teleport.e/issues/7183

Created a label component that accepts any icon that can be rendered on left or right, and have same hover state as buttons depending on kind

Also adds customizing how resource labels are rendered for unified resources

Story for labels: https://localhost:9002/?path=/story/design-label--labels
Story for unified resource: https://localhost:9002/?path=/story/shared-unifiedresources-items--cards

image image

figma: https://www.figma.com/design/CzxaBHF8hirrYv2bTVa4Rw/Identity-Governance?node-id=8638-79995&t=ujyuv9gqrjGLQWfV-0

@kimlisa kimlisa added backport/branch/v18 no-changelog Indicates that a PR does not require a changelog entry labels Dec 5, 2025
@github-actions github-actions bot requested review from avatus and kiosion December 5, 2025 23:10
@kimlisa kimlisa requested review from nicholasmarais1158 and removed request for kiosion December 5, 2025 23:10
@kimlisa kimlisa marked this pull request as draft December 5, 2025 23:11
@kimlisa kimlisa force-pushed the lisa/label-with-button-icon branch from b107bc1 to deb85ce Compare December 6, 2025 00:31
@kimlisa kimlisa force-pushed the lisa/label-with-button-icon branch from deb85ce to 14ab9c7 Compare December 6, 2025 00:35
@kimlisa kimlisa marked this pull request as ready for review December 6, 2025 00:39
Comment on lines +10 to +20
export function LabelButtonWithIcon({
Icon,
placement,
children,
...labelProps
}: LabelProps & {
Icon: React.ComponentType<IconProps>;
placement: IconPlacement;
children: React.ReactNode;
onClick?: () => void;
}) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Use PropsWithChildren.

Suggested change
export function LabelButtonWithIcon({
Icon,
placement,
children,
...labelProps
}: LabelProps & {
Icon: React.ComponentType<IconProps>;
placement: IconPlacement;
children: React.ReactNode;
onClick?: () => void;
}) {
export function LabelButtonWithIcon({
Icon,
placement,
children,
...labelProps
}: {
Icon: React.ComponentType<IconProps>;
placement: IconPlacement;
onClick?: () => void;
} & LabelProps & PropsWithChildren) {

);

return (
<LabelWithHoverAffect {...labelProps} withHoverState>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Support keyboard navigation.

Suggested change
<LabelWithHoverAffect {...labelProps} withHoverState>
<LabelWithHoverAffect {...labelProps} withHoverState tabIndex={0}>

Also, consider adding :focus and/or :active styles.

Comment on lines +16 to +17
Icon: React.ComponentType<IconProps>;
placement: IconPlacement;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To improve flexibility and promote reusability, I'd provide IconLeft and IconRight props instead of a single placement.

Comment on lines +295 to +306
if (resourceLabelConfig?.Icon) {
return (
<StyledLabelButton
key={i}
{...sharedProps}
Icon={resourceLabelConfig.Icon}
placement={resourceLabelConfig.iconPlacement ?? 'right'}
>
{labelText}
</StyledLabelButton>
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels a little strange to base this logic on the presence of an icon. Can it be based on the presence of the onClick prop instead, which should use the hover state?

Comment on lines -61 to +66
const { formatCreateLabel, ...styles } = others;
const { formatCreateLabel, filterOption, ...styles } = others;
return (
<FieldSelectWrapper {...wrapper} {...styles}>
<SelectCreatable<Opt, IsMulti, Group>
{...base}
filterOption={filterOption}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seem a little out of place in this PR. Is it related in some way?

Comment on lines +234 to +250
export type ResourceLabelConfig = {
/**
* If defined, LabelButtonWithIcon component is used that
* renders icon and applies button hover states.
*
* Default is to use Label component without any hover states.
*/
Icon?: React.ComponentType<IconProps>;
/**
* If field "Icon" is defined, default renders the icon to the right.
*/
iconPlacement?: IconPlacement;
/**
* Default label kind is "secondary".
*/
kind?: LabelKind;
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit

Suggested change
export type ResourceLabelConfig = {
/**
* If defined, LabelButtonWithIcon component is used that
* renders icon and applies button hover states.
*
* Default is to use Label component without any hover states.
*/
Icon?: React.ComponentType<IconProps>;
/**
* If field "Icon" is defined, default renders the icon to the right.
*/
iconPlacement?: IconPlacement;
/**
* Default label kind is "secondary".
*/
kind?: LabelKind;
};
export type ResourceLabelConfig = Omit<ComponentProps<typeof LabelButtonWithIcon>, 'children'>;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/branch/v18 no-changelog Indicates that a PR does not require a changelog entry size/md ui

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants