Skip to content

Commit e18471d

Browse files
authored
fix(compact-select): Prevent CompactSelect from stealing focus back to itself after clicking into another element (#72883)
In certain situations, clicking out of an open `CompactSelect` would steal focus back to itself.
1 parent 85b5902 commit e18471d

File tree

5 files changed

+25
-8
lines changed

5 files changed

+25
-8
lines changed

static/app/components/compactSelect/control.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ export function Control({
247247
children,
248248
...wrapperProps
249249
}: ControlProps) {
250+
const wrapperRef = useRef<HTMLDivElement>(null);
250251
// Set up list states (in composite selects, each region has its own state, that way
251252
// selection values are contained within each region).
252253
const [listStates, setListStates] = useState<ListState<any>[]>([]);
@@ -358,7 +359,14 @@ export function Control({
358359
setSearchInputValue('');
359360
setSearch('');
360361

361-
triggerRef.current?.focus();
362+
// Only restore focus if it's already here or lost to the body.
363+
// This prevents focus from being stolen from other elements.
364+
if (
365+
document.activeElement === document.body ||
366+
wrapperRef.current?.contains(document.activeElement)
367+
) {
368+
triggerRef.current?.focus();
369+
}
362370
});
363371
},
364372
});

static/app/components/compactSelect/index.spec.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,9 @@ describe('CompactSelect', function () {
7676
await waitFor(() => {
7777
expect(screen.queryByRole('option', {name: 'Option One'})).not.toBeInTheDocument();
7878
});
79+
await waitFor(() => {
80+
expect(screen.getByRole('button', {name: 'Option One'})).toHaveFocus();
81+
});
7982

8083
// Can be dismissed by pressing Escape
8184
await userEvent.click(screen.getByRole('button', {name: 'Option One'}));
@@ -89,14 +92,17 @@ describe('CompactSelect', function () {
8992
await waitFor(() => {
9093
expect(screen.queryByRole('option', {name: 'Option One'})).not.toBeInTheDocument();
9194
});
95+
await waitFor(() => {
96+
expect(screen.getByRole('button', {name: 'Option One'})).toHaveFocus();
97+
});
9298

9399
// When menu A is open, clicking once on menu B's trigger button closes menu A and
94100
// then opens menu B
95101
await userEvent.click(screen.getByRole('button', {name: 'Option One'}));
96102
expect(screen.getByRole('option', {name: 'Option One'})).toBeInTheDocument();
97103
await userEvent.click(screen.getByRole('button', {name: 'Option Three'}));
98104
expect(screen.queryByRole('option', {name: 'Option One'})).not.toBeInTheDocument();
99-
expect(screen.getByRole('option', {name: 'Option Three'})).toBeInTheDocument();
105+
expect(await screen.findByRole('option', {name: 'Option Three'})).toBeInTheDocument();
100106
});
101107

102108
describe('ListBox', function () {

static/app/components/events/eventTags/eventTagsTree.spec.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,10 @@ describe('EventTagsTree', function () {
106106
for (const link of linkDropdowns) {
107107
await userEvent.click(link);
108108
expect(
109-
screen.getByLabelText('View issues with this tag value')
109+
await screen.findByLabelText('View issues with this tag value')
110110
).toBeInTheDocument();
111111
expect(
112-
screen.getByLabelText('View other events with this tag value')
112+
await screen.findByLabelText('View other events with this tag value')
113113
).toBeInTheDocument();
114114
expect(screen.getByLabelText('Copy tag value to clipboard')).toBeInTheDocument();
115115
}

static/app/components/events/highlights/highlightsDataSection.spec.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ describe('HighlightsDataSection', function () {
9090
expect(highlightTagDropdown).toBeInTheDocument();
9191
await userEvent.click(highlightTagDropdown);
9292
expect(
93-
screen.getByLabelText('View issues with this tag value')
93+
await screen.findByLabelText('View issues with this tag value')
9494
).toBeInTheDocument();
9595
expect(
9696
screen.queryByLabelText('Add to event highlights')

static/app/utils/useOverlay.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -250,10 +250,13 @@ function useOverlay({
250250
if (interactedOutside.current) {
251251
onInteractOutside?.();
252252
interactedOutside.current = false;
253-
254-
interactOutsideTrigger.current?.focus();
255-
interactOutsideTrigger.current?.click();
253+
const trigger = interactOutsideTrigger.current;
256254
interactOutsideTrigger.current = null;
255+
256+
requestAnimationFrame(() => {
257+
trigger?.focus();
258+
trigger?.click();
259+
});
257260
}
258261

259262
openState.close();

0 commit comments

Comments
 (0)