From 804b7d3262fcb87760b3c800b71266f987d3cbb4 Mon Sep 17 00:00:00 2001 From: Liu Liu Date: Wed, 25 Jun 2025 15:50:50 -0700 Subject: [PATCH 1/6] add loading prop --- .changeset/calm-hoops-tie.md | 5 +++++ packages/react/src/ActionList/ActionList.docs.json | 8 +++++++- packages/react/src/ActionList/TrailingAction.tsx | 8 +++++++- 3 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 .changeset/calm-hoops-tie.md diff --git a/.changeset/calm-hoops-tie.md b/.changeset/calm-hoops-tie.md new file mode 100644 index 00000000000..79b4bc889f8 --- /dev/null +++ b/.changeset/calm-hoops-tie.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Add loading support to ActionList.TrailingAction component. diff --git a/packages/react/src/ActionList/ActionList.docs.json b/packages/react/src/ActionList/ActionList.docs.json index 6711f0caf31..73a6e036560 100644 --- a/packages/react/src/ActionList/ActionList.docs.json +++ b/packages/react/src/ActionList/ActionList.docs.json @@ -251,6 +251,12 @@ "name": "href", "type": "string", "description": "href when the TrailingAction is rendered as a link." + }, + { + "name": "loading", + "type": "boolean", + "defaultValue": "false", + "description": "Whether the TrailingAction is in a loading state. When true, the TrailingAction will render a spinner instead of an icon." } ] }, @@ -393,4 +399,4 @@ ] } ] -} +} \ No newline at end of file diff --git a/packages/react/src/ActionList/TrailingAction.tsx b/packages/react/src/ActionList/TrailingAction.tsx index 4b17748e5a6..c2fca5d7df0 100644 --- a/packages/react/src/ActionList/TrailingAction.tsx +++ b/packages/react/src/ActionList/TrailingAction.tsx @@ -19,10 +19,14 @@ export type ActionListTrailingActionProps = ElementProps & { icon?: React.ElementType label: string className?: string + /** + * Specify whether the action is in a loading state + */ + loading?: boolean } export const TrailingAction = forwardRef( - ({as = 'button', icon, label, href = null, className, ...props}, forwardedRef) => { + ({as = 'button', icon, label, href = null, className, loading, ...props}, forwardedRef) => { return ( {icon ? ( @@ -33,6 +37,7 @@ export const TrailingAction = forwardRef( variant="invisible" tooltipDirection="w" href={href} + loading={loading} // @ts-expect-error StyledButton wants both Anchor and Button refs ref={forwardedRef} className={classes.TrailingActionButton} @@ -44,6 +49,7 @@ export const TrailingAction = forwardRef( variant="invisible" as={as} href={href} + loading={loading} ref={forwardedRef} className={classes.TrailingActionButton} {...props} From 08511be2720ccdb445f3f7442401e020e9eddf46 Mon Sep 17 00:00:00 2001 From: Liu Liu Date: Mon, 30 Jun 2025 10:43:23 -0700 Subject: [PATCH 2/6] adjust the loading button ui, restrict loading to buttons, uncomment stories --- .../ActionList.features.stories.tsx | 154 +++++++++++------- .../src/ActionList/ActionList.module.css | 35 +++- .../react/src/ActionList/TrailingAction.tsx | 12 +- 3 files changed, 133 insertions(+), 68 deletions(-) diff --git a/packages/react/src/ActionList/ActionList.features.stories.tsx b/packages/react/src/ActionList/ActionList.features.stories.tsx index 20daafcfbd3..72bee7b9a83 100644 --- a/packages/react/src/ActionList/ActionList.features.stories.tsx +++ b/packages/react/src/ActionList/ActionList.features.stories.tsx @@ -827,64 +827,102 @@ export const WithCustomTrailingVisuals = () => ( ) -// removing this until CSS Modules FF ships, currently broken in production if button semantic FF is false -// export const WithTrailingAction = () => { -// return ( -// -// -// -// -// -// -// Item 1 (with default TrailingAction) -// -// -// -// Item 2 (with link TrailingAction) -// -// -// -// Item 3This is an inline description. -// -// -// -// Item 4This is a block description. -// -// -// -// Item 5This is a block description. -// -// -// -// Item 6 -// -// -// -// LinkItem 1 -// -// with TrailingAction this is a long description and should not cause horizontal scroll on smaller screen -// sizes -// -// -// -// -// LinkItem 2 -// -// with TrailingVisual this is a long description and should not cause horizontal scroll on smaller screen -// sizes -// -// -// -// -// -// -// Inactive ItemWith TrailingAction -// -// -// -// -// ) -// } +export const WithTrailingAction = () => { + const [loadingState, setLoadingState] = React.useState(false) + + // Auto-toggle every 2.5 seconds to continuously show transitions + React.useEffect(() => { + const interval = setInterval(() => { + setLoadingState(prev => !prev) + }, 2500) + + return () => clearInterval(interval) + }, []) + + return ( + + + + + + + Item 1 (with default TrailingAction) + + + + Item 2 (with link TrailingAction) + + + + Item 3This is an inline description. + + + + Item 4This is a block description. + + + + Item 5This is a block description. + + + + Item 6 + + + + Icon button loading state + + Shows how IconButton maintains width and centers spinner when loading + + + + + Icon button with transitions + + Automatically toggles loading state every 2.5 seconds to show transitions + + + + + Text button loading state + + Shows how text button aligns spinner to the right and preserves width + + + + + Text button with transitions + + Automatically toggles loading state every 2.5 seconds to show transitions + + + + + LinkItem 1 + + with TrailingAction this is a long description and should not cause horizontal scroll on smaller screen + sizes + + + + + LinkItem 2 + + with TrailingVisual this is a long description and should not cause horizontal scroll on smaller screen + sizes + + + + + + + Inactive ItemWith TrailingAction + + + + + ) +} export const FullVariant = () => ( diff --git a/packages/react/src/ActionList/ActionList.module.css b/packages/react/src/ActionList/ActionList.module.css index b5fec848a47..7f46942ea24 100644 --- a/packages/react/src/ActionList/ActionList.module.css +++ b/packages/react/src/ActionList/ActionList.module.css @@ -107,7 +107,7 @@ :focus, &:focus-visible, /* stylelint-disable-next-line selector-no-qualifying-type */ - > a.focus-visible, + >a.focus-visible, &[data-is-active-descendant] { /* Support for Windows high contrast https://sarahmhigley.com/writing/whcm-quick-tips */ outline: solid 1px transparent !important; @@ -364,7 +364,8 @@ border-radius: var(--borderRadius-small); transition: background-color, - border-color 80ms cubic-bezier(0.33, 1, 0.68, 1); /* checked -> unchecked - add 120ms delay to fully see animation-out */ + border-color 80ms cubic-bezier(0.33, 1, 0.68, 1); + /* checked -> unchecked - add 120ms delay to fully see animation-out */ place-content: center; @@ -382,7 +383,8 @@ mask-size: 75%; mask-repeat: no-repeat; mask-position: center; - animation: checkmarkOut 80ms cubic-bezier(0.65, 0, 0.35, 1); /* forwards; slightly snappier animation out */ + animation: checkmarkOut 80ms cubic-bezier(0.65, 0, 0.35, 1); + /* forwards; slightly snappier animation out */ } @media (forced-colors: active) { @@ -400,7 +402,8 @@ border-color: var(--control-checked-borderColor-rest); transition: background-color, - border-color 80ms cubic-bezier(0.32, 0, 0.67, 0) 0ms; /* unchecked -> checked */ + border-color 80ms cubic-bezier(0.32, 0, 0.67, 0) 0ms; + /* unchecked -> checked */ &::before { visibility: visible; @@ -617,7 +620,8 @@ span wrapping svg or text */ min-width: max-content; min-height: var(--control-medium-lineBoxHeight); /* stylelint-disable-next-line primer/typography */ - line-height: 20px; /* temporary until we fix line-height rounding in primitives */ + line-height: 20px; + /* temporary until we fix line-height rounding in primitives */ color: var(--fgColor-muted); pointer-events: none; fill: var(--fgColor-muted); @@ -630,7 +634,8 @@ span wrapping svg or text */ font-size: var(--text-body-size-medium); font-weight: var(--base-text-weight-normal); /* stylelint-disable-next-line primer/typography */ - line-height: 20px; /* temporary until we fix line-height rounding in primitives */ + line-height: 20px; + /* temporary until we fix line-height rounding in primitives */ color: var(--fgColor-default); grid-area: label; /* stylelint-disable-next-line declaration-property-value-keyword-no-deprecated */ @@ -652,6 +657,24 @@ span wrapping svg or text */ .TrailingActionButton { border-top-left-radius: 0; border-bottom-left-radius: 0; + + /* Preserve width consistency when loading state is active for text buttons only */ + &[data-loading='true']:has([data-component='buttonContent']) { + /* Double the left padding to compensate for missing right padding */ + padding: 0 0 0 calc(var(--control-medium-paddingInline-normal, 0.75rem) * 2); + + /* Position spinner at the end to align with IconButton */ + & [data-component='loadingSpinner'] { + place-self: end; + /* Match the IconButton spinner size */ + width: var(--control-medium-size, 2rem); + height: var(--control-medium-size, 2rem); + /* Ensure spinner is properly centered */ + display: flex; + align-items: center; + justify-content: center; + } + } } .InactiveButtonWrap { diff --git a/packages/react/src/ActionList/TrailingAction.tsx b/packages/react/src/ActionList/TrailingAction.tsx index c2fca5d7df0..d974bd0ab9a 100644 --- a/packages/react/src/ActionList/TrailingAction.tsx +++ b/packages/react/src/ActionList/TrailingAction.tsx @@ -9,20 +9,22 @@ type ElementProps = | { as?: 'button' href?: never + /** + * Specify whether the action is in a loading state. + * Only available for button elements. + */ + loading?: boolean } | { as: 'a' href: string + loading?: never } export type ActionListTrailingActionProps = ElementProps & { icon?: React.ElementType label: string className?: string - /** - * Specify whether the action is in a loading state - */ - loading?: boolean } export const TrailingAction = forwardRef( @@ -38,6 +40,7 @@ export const TrailingAction = forwardRef( tooltipDirection="w" href={href} loading={loading} + data-loading={Boolean(loading)} // @ts-expect-error StyledButton wants both Anchor and Button refs ref={forwardedRef} className={classes.TrailingActionButton} @@ -50,6 +53,7 @@ export const TrailingAction = forwardRef( as={as} href={href} loading={loading} + data-loading={Boolean(loading)} ref={forwardedRef} className={classes.TrailingActionButton} {...props} From 196b8c4505efd4cce9ac63cb3ec680b5e93aba86 Mon Sep 17 00:00:00 2001 From: Liu Liu Date: Mon, 30 Jun 2025 12:14:03 -0700 Subject: [PATCH 3/6] use base-size-12 --- packages/react/src/ActionList/ActionList.module.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/ActionList/ActionList.module.css b/packages/react/src/ActionList/ActionList.module.css index 7f46942ea24..31b94b58eae 100644 --- a/packages/react/src/ActionList/ActionList.module.css +++ b/packages/react/src/ActionList/ActionList.module.css @@ -661,7 +661,7 @@ span wrapping svg or text */ /* Preserve width consistency when loading state is active for text buttons only */ &[data-loading='true']:has([data-component='buttonContent']) { /* Double the left padding to compensate for missing right padding */ - padding: 0 0 0 calc(var(--control-medium-paddingInline-normal, 0.75rem) * 2); + padding: 0 0 0 calc(var(--base-size-12) * 2); /* Position spinner at the end to align with IconButton */ & [data-component='loadingSpinner'] { From 783d1d54eb869c0dea525ed47a11e8e09abb17a6 Mon Sep 17 00:00:00 2001 From: Liu Liu Date: Mon, 30 Jun 2025 16:23:03 -0700 Subject: [PATCH 4/6] maintain accessible contrast for labels when TrailingAction is loading --- .../src/ActionList/ActionList.module.css | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/react/src/ActionList/ActionList.module.css b/packages/react/src/ActionList/ActionList.module.css index d7b5679e3c8..b37f27c2445 100644 --- a/packages/react/src/ActionList/ActionList.module.css +++ b/packages/react/src/ActionList/ActionList.module.css @@ -301,8 +301,8 @@ /* disabled */ - &[aria-disabled='true'], - &:has([aria-disabled='true'], [disabled]) { + /* When the entire item is disabled, everything should appear disabled */ + &[aria-disabled='true'] { & .ActionListContent * { color: var(--control-fgColor-disabled); } @@ -342,6 +342,19 @@ } } + /* When only child elements are disabled (e.g., loading TrailingAction), + keep labels and descriptions accessible */ + &:has([aria-disabled='true'], [disabled]):not([aria-disabled='true']) { + /* Ensure labels and descriptions maintain accessibility contrast */ + & .ItemLabel { + color: var(--fgColor-default); + } + + & .Description { + color: var(--fgColor-default); + } + } + /* Make sure that the first visible item isn't a divider */ &[aria-hidden] + .Divider { display: none; @@ -616,7 +629,7 @@ default block */ /* remove after FF ships */ /* stylelint-disable-next-line primer/typography */ line-height: 16px; - color: var(--fgColor-muted); + color: var(--fgColor-default); } /* helper for grid alignment with multi-line content From d84e56bd71461eb987c94e488834e79f8965a866 Mon Sep 17 00:00:00 2001 From: Liu Liu Date: Mon, 30 Jun 2025 17:23:51 -0700 Subject: [PATCH 5/6] improve color contrast when trailingaction is loading --- packages/react/src/ActionList/ActionList.docs.json | 2 +- packages/react/src/ActionList/ActionList.module.css | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/react/src/ActionList/ActionList.docs.json b/packages/react/src/ActionList/ActionList.docs.json index 4061ab7e762..2824342dad7 100644 --- a/packages/react/src/ActionList/ActionList.docs.json +++ b/packages/react/src/ActionList/ActionList.docs.json @@ -256,7 +256,7 @@ "name": "loading", "type": "boolean", "defaultValue": "false", - "description": "Whether the TrailingAction is in a loading state. When true, the TrailingAction will render a spinner instead of an icon." + "description": "Whether the TrailingAction is in a loading state. When true, the TrailingAction will render a spinner instead of an icon. Only available when `as` is 'button'." } ] }, diff --git a/packages/react/src/ActionList/ActionList.module.css b/packages/react/src/ActionList/ActionList.module.css index b37f27c2445..d252888491f 100644 --- a/packages/react/src/ActionList/ActionList.module.css +++ b/packages/react/src/ActionList/ActionList.module.css @@ -301,8 +301,8 @@ /* disabled */ - /* When the entire item is disabled, everything should appear disabled */ - &[aria-disabled='true'] { + &[aria-disabled='true'], + &:has([aria-disabled='true'], [disabled]) { & .ActionListContent * { color: var(--control-fgColor-disabled); } @@ -342,9 +342,8 @@ } } - /* When only child elements are disabled (e.g., loading TrailingAction), - keep labels and descriptions accessible */ - &:has([aria-disabled='true'], [disabled]):not([aria-disabled='true']) { + /* When TrailingAction is in loading state, keep labels and descriptions accessible */ + &:has(.TrailingAction [data-loading='true']):not([aria-disabled='true']) { /* Ensure labels and descriptions maintain accessibility contrast */ & .ItemLabel { color: var(--fgColor-default); From 5f4fecc3fb0399a4a82a25fbe24e55c4c5c3c948 Mon Sep 17 00:00:00 2001 From: Liu Liu Date: Mon, 30 Jun 2025 17:35:46 -0700 Subject: [PATCH 6/6] muted --- packages/react/src/ActionList/ActionList.module.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/ActionList/ActionList.module.css b/packages/react/src/ActionList/ActionList.module.css index d252888491f..376a96b7844 100644 --- a/packages/react/src/ActionList/ActionList.module.css +++ b/packages/react/src/ActionList/ActionList.module.css @@ -628,7 +628,7 @@ default block */ /* remove after FF ships */ /* stylelint-disable-next-line primer/typography */ line-height: 16px; - color: var(--fgColor-default); + color: var(--fgColor-muted); } /* helper for grid alignment with multi-line content