Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions static/app/components/compactSelect/composite.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,16 @@ describe('CompactSelect', function () {

// press arrow up and second option in the first region gets focus
await userEvent.keyboard('{ArrowUp}');
expect(screen.getByRole('option', {name: 'Choice Two'})).toHaveFocus();
await waitFor(() => {
expect(screen.getByRole('option', {name: 'Choice Two'})).toHaveFocus();
});

// press arrow down 3 times and focus moves to the third and fourth option, before
// wrapping back to the first option
await userEvent.keyboard('{ArrowDown>3}');
expect(screen.getByRole('option', {name: 'Choice One'})).toHaveFocus();
await waitFor(() => {
expect(screen.getByRole('option', {name: 'Choice One'})).toHaveFocus();
});
});

it('has separate, async self-contained select regions', async function () {
Expand Down
78 changes: 44 additions & 34 deletions static/app/components/compactSelect/control.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,20 @@ import usePrevious from 'sentry/utils/usePrevious';
import type {SingleListProps} from './list';
import type {SelectOption} from './types';

// autoFocus react attribute is sync called on render, this causes
// layout thrashing and is bad for performance. This thin wrapper function
// will defer the focus call until the next frame, after the browser and react
// have had a chance to update the DOM, splitting the perf cost across frames.
function nextFrameCallback(cb: () => void) {
if ('requestAnimationFrame' in window) {
window.requestAnimationFrame(() => cb());
} else {
setTimeout(() => {
cb();
}, 1);
}
}

export interface SelectContextValue {
overlayIsOpen: boolean;
/**
Expand Down Expand Up @@ -311,45 +325,41 @@ export function Control({
shouldCloseOnBlur,
preventOverflowOptions,
flipOptions,
onOpenChange: async open => {
// On open
if (open) {
// Wait for overlay to appear/disappear
await new Promise(resolve => resolve(null));

// Focus on search box if present
if (searchable) {
searchRef.current?.focus();
return;
}

const firstSelectedOption = overlayRef.current?.querySelector<HTMLLIElement>(
`li[role="${grid ? 'row' : 'option'}"][aria-selected="true"]`
);

// Focus on first selected item
if (firstSelectedOption) {
firstSelectedOption.focus();
onOpenChange: open => {
nextFrameCallback(() => {
if (open) {
// Focus on search box if present
if (searchable) {
searchRef.current?.focus();
return;
}

const firstSelectedOption = overlayRef.current?.querySelector<HTMLLIElement>(
`li[role="${grid ? 'row' : 'option'}"][aria-selected="true"]`
);

// Focus on first selected item
if (firstSelectedOption) {
firstSelectedOption.focus();
return;
}

// If no item is selected, focus on first item instead
overlayRef.current
?.querySelector<HTMLLIElement>(`li[role="${grid ? 'row' : 'option'}"]`)
?.focus();
return;
}

// If no item is selected, focus on first item instead
overlayRef.current
?.querySelector<HTMLLIElement>(`li[role="${grid ? 'row' : 'option'}"]`)
?.focus();
return;
}

// On close
onClose?.();
// On close
onClose?.();

// Clear search string
setSearchInputValue('');
setSearch('');
// Clear search string
setSearchInputValue('');
setSearch('');

// Wait for overlay to appear/disappear
await new Promise(resolve => resolve(null));
triggerRef.current?.focus();
triggerRef.current?.focus();
});
},
});

Expand Down
37 changes: 30 additions & 7 deletions static/app/components/compactSelect/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,22 @@ describe('CompactSelect', function () {
await userEvent.click(screen.getByRole('button', {name: 'Option One'}));
expect(screen.getByRole('option', {name: 'Option One'})).toBeInTheDocument();
await userEvent.click(document.body);
expect(screen.queryByRole('option', {name: 'Option One'})).not.toBeInTheDocument();
await waitFor(() => {
expect(screen.queryByRole('option', {name: 'Option One'})).not.toBeInTheDocument();
});

// Can be dismissed by pressing Escape
await userEvent.click(screen.getByRole('button', {name: 'Option One'}));
expect(screen.getByRole('option', {name: 'Option One'})).toBeInTheDocument();
await waitFor(() => {
expect(screen.getByRole('option', {name: 'Option One'})).toBeInTheDocument();
});
await waitFor(() => {
expect(screen.getByRole('option', {name: 'Option One'})).toHaveFocus();
});
await userEvent.keyboard('{Escape}');
expect(screen.queryByRole('option', {name: 'Option One'})).not.toBeInTheDocument();
await waitFor(() => {
expect(screen.queryByRole('option', {name: 'Option One'})).not.toBeInTheDocument();
});

// When menu A is open, clicking once on menu B's trigger button closes menu A and
// then opens menu B
Expand Down Expand Up @@ -230,11 +239,18 @@ describe('CompactSelect', function () {
// there's a message prompting the user to use search to find more options
expect(screen.getByText('Use search for more options…')).toBeInTheDocument();

await waitFor(() => {
expect(screen.getByPlaceholderText('Search…')).toHaveFocus();
});
// Option Three is not reachable via keyboard, focus wraps back to Option One
await userEvent.keyboard(`{ArrowDown}`);
expect(screen.getByRole('option', {name: 'Option One'})).toHaveFocus();
await waitFor(() => {
expect(screen.getByRole('option', {name: 'Option One'})).toHaveFocus();
});
await userEvent.keyboard(`{ArrowDown>2}`);
expect(screen.getByRole('option', {name: 'Option One'})).toHaveFocus();
await waitFor(() => {
expect(screen.getByRole('option', {name: 'Option One'})).toHaveFocus();
});

// Option Three is still available via search
await userEvent.type(screen.getByPlaceholderText('Search…'), 'three');
Expand Down Expand Up @@ -371,7 +387,9 @@ describe('CompactSelect', function () {

// close the menu
await userEvent.click(document.body);
expect(onCloseMock).toHaveBeenCalled();
await waitFor(() => {
expect(onCloseMock).toHaveBeenCalled();
});
});
});

Expand Down Expand Up @@ -520,6 +538,9 @@ describe('CompactSelect', function () {
// there's a message prompting the user to use search to find more options
expect(screen.getByText('Use search for more options…')).toBeInTheDocument();

await waitFor(() => {
expect(screen.getByPlaceholderText('Search…')).toHaveFocus();
});
// Option Three is not reachable via keyboard, focus wraps back to Option One
await userEvent.keyboard(`{ArrowDown}`);
expect(screen.getByRole('row', {name: 'Option One'})).toHaveFocus();
Expand Down Expand Up @@ -663,7 +684,9 @@ describe('CompactSelect', function () {

// close the menu
await userEvent.click(document.body);
expect(onCloseMock).toHaveBeenCalled();
await waitFor(() => {
expect(onCloseMock).toHaveBeenCalled();
});
});

it('allows keyboard navigation to nested buttons', async function () {
Expand Down
20 changes: 16 additions & 4 deletions static/app/components/organizations/hybridFilter.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import {fireEvent, render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
import {
fireEvent,
render,
screen,
userEvent,
waitFor,
} from 'sentry-test/reactTestingLibrary';

import {HybridFilter} from 'sentry/components/organizations/hybridFilter';

Expand Down Expand Up @@ -155,15 +161,21 @@ describe('ProjectPageFilter', function () {

// Open the menu, focus is on search input
await userEvent.click(screen.getByRole('button', {expanded: false}));
expect(screen.getByPlaceholderText('Search…')).toHaveFocus();
await waitFor(() => {
expect(screen.getByPlaceholderText('Search…')).toHaveFocus();
});

// Press Arrow Down to move focus to Option One
await userEvent.keyboard('{ArrowDown}');
expect(screen.getByRole('row', {name: 'Option One'})).toHaveFocus();
await waitFor(() => {
expect(screen.getByRole('row', {name: 'Option One'})).toHaveFocus();
});

// Press Arrow Right to move focus to the checkbox
await userEvent.keyboard('{ArrowRight}');
expect(screen.getByRole('checkbox', {name: 'Select Option One'})).toHaveFocus();
await waitFor(() => {
expect(screen.getByRole('checkbox', {name: 'Select Option One'})).toHaveFocus();
});

// Activate the checkbox. In browsers, users can press Space when the checkbox is
// focused to activate it. With RTL, however, onChange events aren't fired on Space
Expand Down
16 changes: 9 additions & 7 deletions static/app/views/dashboards/detail.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1105,13 +1105,15 @@ describe('Dashboards > Detail', function () {
screen.getByText('All Releases');
await userEvent.click(document.body);

expect(browserHistory.push).toHaveBeenCalledWith(
expect.objectContaining({
query: expect.objectContaining({
release: '',
}),
})
);
await waitFor(() => {
expect(browserHistory.push).toHaveBeenCalledWith(
expect.objectContaining({
query: expect.objectContaining({
release: '',
}),
})
);
});
});

it('can save absolute time range in existing dashboard', async () => {
Expand Down