From 6db57dc7fb77e0c9be6142b10f0adb7baa190039 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Mon, 9 Mar 2026 12:02:58 +0100 Subject: [PATCH 1/6] feat: Busy spinner with absolute position --- static/app/components/badge/groupPriority.tsx | 22 +++++-------- .../components/core/button/button.spec.tsx | 28 ++++++++++++++++ static/app/components/core/button/button.tsx | 33 +++++++++++++++++++ .../app/components/core/layout/container.tsx | 4 +++ 4 files changed, 74 insertions(+), 13 deletions(-) diff --git a/static/app/components/badge/groupPriority.tsx b/static/app/components/badge/groupPriority.tsx index b57ce7809be134..00622e02a288de 100644 --- a/static/app/components/badge/groupPriority.tsx +++ b/static/app/components/badge/groupPriority.tsx @@ -247,19 +247,6 @@ export function GroupPriorityDropdown({ ); } -const DropdownButton = styled(Button)` - font-weight: ${p => p.theme.font.weight.sans.regular}; - border: none; - padding: 0; - height: unset; - border-radius: 20px; - box-shadow: none; - - > span > div { - border-radius: 20px; - } -`; - const StyledTag = styled(Tag)` gap: ${p => p.theme.space['2xs']}; position: relative; @@ -267,6 +254,15 @@ const StyledTag = styled(Tag)` overflow: hidden; `; +const DropdownButton = styled(Button)` + padding: 0; + border-radius: ${p => p.theme.radius.full}; + + ${StyledTag} { + border-radius: ${p => p.theme.radius.full}; + } +`; + const InlinePlaceholder = styled(Placeholder)` display: inline-block; vertical-align: middle; diff --git a/static/app/components/core/button/button.spec.tsx b/static/app/components/core/button/button.spec.tsx index fb99d7c1d8c592..03aa4ec7402ace 100644 --- a/static/app/components/core/button/button.spec.tsx +++ b/static/app/components/core/button/button.spec.tsx @@ -26,6 +26,34 @@ describe('Button', () => { expect(spy).not.toHaveBeenCalled(); }); + + it('does not call `onClick` on busy buttons', async () => { + const spy = jest.fn(); + render( + + ); + await userEvent.click(screen.getByText('Click me')); + + expect(spy).not.toHaveBeenCalled(); + }); + + it('shows spinner when busy', () => { + render(); + + const button = screen.getByRole('button', {name: 'Busy Button'}); + expect(button).toHaveAttribute('aria-busy', 'true'); + expect(screen.getByRole('status', {name: 'Busy'})).not.toHaveStyle({ + visibility: 'hidden', + }); + }); + + it('hides spinner when not busy', () => { + render(); + + expect(screen.queryByRole('status', {name: 'Busy'})).not.toBeInTheDocument(); + }); }); describe('LinkButton', () => { diff --git a/static/app/components/core/button/button.tsx b/static/app/components/core/button/button.tsx index b84cf750c79b20..1b3e18543ff00a 100644 --- a/static/app/components/core/button/button.tsx +++ b/static/app/components/core/button/button.tsx @@ -1,3 +1,4 @@ +import {keyframes} from '@emotion/react'; import styled from '@emotion/styled'; import {Flex} from '@sentry/scraps/layout'; @@ -55,6 +56,7 @@ export function Button({ minWidth="0" height="100%" whiteSpace="nowrap" + visibility={busy ? 'hidden' : undefined} > {props.icon && ( )} {props.children} + {busy && ( + + {({className}) => ( + + )} + + )} @@ -80,3 +94,22 @@ export function Button({ const StyledButton = styled('button')` ${p => getButtonStyles(p as any)} `; + +const spin = keyframes` + to { + transform: rotate(360deg); + } +`; + +const BusySpinner = styled('span')` + &::after { + content: ''; + display: block; + width: 1em; + height: 1em; + border-radius: 50%; + border: 2px solid currentColor; + border-top-color: transparent; + animation: ${spin} 0.6s linear infinite; + } +`; diff --git a/static/app/components/core/layout/container.tsx b/static/app/components/core/layout/container.tsx index f90b5690465b31..8fc5f30f53608f 100644 --- a/static/app/components/core/layout/container.tsx +++ b/static/app/components/core/layout/container.tsx @@ -84,6 +84,8 @@ interface ContainerLayoutProps { alignSelf?: Responsive; justifySelf?: Responsive; + visibility?: Responsive<'visible' | 'hidden' | 'collapse'>; + // Text Wrapping whiteSpace?: Responsive< 'break-spaces' | 'normal' | 'nowrap' | 'pre' | 'pre-line' | 'pre-wrap' @@ -222,6 +224,7 @@ const omitContainerProps = new Set([ 'right', 'row', 'top', + 'visibility', 'width', 'whiteSpace', ]); @@ -309,6 +312,7 @@ export const Container = styled( ${p => rc('border-left', p.borderLeft, p.theme, getBorder)}; ${p => rc('border-right', p.borderRight, p.theme, getBorder)}; + ${p => rc('visibility', p.visibility, p.theme)}; ${p => rc('white-space', p.whiteSpace, p.theme)}; /** From 6811c8943005522b88e4489afe327d47ab560903 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Tue, 10 Mar 2026 09:22:11 +0100 Subject: [PATCH 2/6] ref: move aria-label to button instead of spinner --- static/app/components/core/button/button.spec.tsx | 2 +- static/app/components/core/button/button.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/static/app/components/core/button/button.spec.tsx b/static/app/components/core/button/button.spec.tsx index 03aa4ec7402ace..bfbc7d1297f3b8 100644 --- a/static/app/components/core/button/button.spec.tsx +++ b/static/app/components/core/button/button.spec.tsx @@ -42,7 +42,7 @@ describe('Button', () => { it('shows spinner when busy', () => { render(); - const button = screen.getByRole('button', {name: 'Busy Button'}); + const button = screen.getByRole('button', {name: 'Busy'}); expect(button).toHaveAttribute('aria-busy', 'true'); expect(screen.getByRole('status', {name: 'Busy'})).not.toHaveStyle({ visibility: 'hidden', diff --git a/static/app/components/core/button/button.tsx b/static/app/components/core/button/button.tsx index 1b3e18543ff00a..5265120bfac1f1 100644 --- a/static/app/components/core/button/button.tsx +++ b/static/app/components/core/button/button.tsx @@ -38,7 +38,7 @@ export function Button({ disabled={!tooltipProps?.title} > Date: Tue, 10 Mar 2026 09:33:59 +0100 Subject: [PATCH 3/6] fix: add inset 0 --- static/app/components/core/button/button.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/static/app/components/core/button/button.tsx b/static/app/components/core/button/button.tsx index 5265120bfac1f1..96786d24ead848 100644 --- a/static/app/components/core/button/button.tsx +++ b/static/app/components/core/button/button.tsx @@ -79,6 +79,7 @@ export function Button({ justify="center" position="absolute" visibility="visible" + inset={0} > {({className}) => ( From 548f919c91b368135eb3c545038273c0c5b35548 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Tue, 10 Mar 2026 16:05:32 +0100 Subject: [PATCH 4/6] Revert "ref: move aria-label to button instead of spinner" This reverts commit 6811c8943005522b88e4489afe327d47ab560903. --- static/app/components/core/button/button.spec.tsx | 2 +- static/app/components/core/button/button.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/static/app/components/core/button/button.spec.tsx b/static/app/components/core/button/button.spec.tsx index bfbc7d1297f3b8..03aa4ec7402ace 100644 --- a/static/app/components/core/button/button.spec.tsx +++ b/static/app/components/core/button/button.spec.tsx @@ -42,7 +42,7 @@ describe('Button', () => { it('shows spinner when busy', () => { render(); - const button = screen.getByRole('button', {name: 'Busy'}); + const button = screen.getByRole('button', {name: 'Busy Button'}); expect(button).toHaveAttribute('aria-busy', 'true'); expect(screen.getByRole('status', {name: 'Busy'})).not.toHaveStyle({ visibility: 'hidden', diff --git a/static/app/components/core/button/button.tsx b/static/app/components/core/button/button.tsx index 96786d24ead848..007c7fbe25b405 100644 --- a/static/app/components/core/button/button.tsx +++ b/static/app/components/core/button/button.tsx @@ -38,7 +38,7 @@ export function Button({ disabled={!tooltipProps?.title} > Date: Tue, 10 Mar 2026 16:10:54 +0100 Subject: [PATCH 5/6] fix: make the spinner aria-hidden, as it's decorative --- static/app/components/core/button/button.spec.tsx | 5 ++--- static/app/components/core/button/button.tsx | 4 +--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/static/app/components/core/button/button.spec.tsx b/static/app/components/core/button/button.spec.tsx index 03aa4ec7402ace..53348cefdc9009 100644 --- a/static/app/components/core/button/button.spec.tsx +++ b/static/app/components/core/button/button.spec.tsx @@ -44,9 +44,8 @@ describe('Button', () => { const button = screen.getByRole('button', {name: 'Busy Button'}); expect(button).toHaveAttribute('aria-busy', 'true'); - expect(screen.getByRole('status', {name: 'Busy'})).not.toHaveStyle({ - visibility: 'hidden', - }); + const spinner = button.querySelector('[aria-hidden="true"]'); + expect(spinner).toBeInTheDocument(); }); it('hides spinner when not busy', () => { diff --git a/static/app/components/core/button/button.tsx b/static/app/components/core/button/button.tsx index 007c7fbe25b405..5ece056c82a57a 100644 --- a/static/app/components/core/button/button.tsx +++ b/static/app/components/core/button/button.tsx @@ -81,9 +81,7 @@ export function Button({ visibility="visible" inset={0} > - {({className}) => ( - - )} + {({className}) => } )} From c7d2430d27519afc465054181c5fa063c035bd38 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Tue, 10 Mar 2026 16:27:32 +0100 Subject: [PATCH 6/6] ref: improve test --- static/app/components/core/button/button.spec.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/static/app/components/core/button/button.spec.tsx b/static/app/components/core/button/button.spec.tsx index 53348cefdc9009..1ed0e958195088 100644 --- a/static/app/components/core/button/button.spec.tsx +++ b/static/app/components/core/button/button.spec.tsx @@ -51,7 +51,11 @@ describe('Button', () => { it('hides spinner when not busy', () => { render(); - expect(screen.queryByRole('status', {name: 'Busy'})).not.toBeInTheDocument(); + const button = screen.getByRole('button', {name: 'Normal Button'}); + expect(button).not.toHaveAttribute('aria-busy'); + + const spinner = button.querySelector('[aria-hidden="true"]'); + expect(spinner).not.toBeInTheDocument(); }); });