Skip to content

Commit c5618d5

Browse files
rusackasclaude
andcommitted
fix(test): stop Escape from invalidating the Tab propagation check
Escape legitimately closes the modal (rc-component/portal's global ESC handler unmounts it), so firing it before Tab left the input detached and made the Tab assertion fail. Check Tab first, then Escape. Also extract the repeated `.ant-select-content-has-value[title=...], .ant-select-selection-item[title=...]` selector into a helper per bito-code-review's suggestion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent da55a57 commit c5618d5

2 files changed

Lines changed: 17 additions & 14 deletions

File tree

superset-frontend/packages/superset-ui-core/src/components/ModalTrigger/ModalTrigger.test.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,11 @@ test('stops propagation of navigation keys to parent elements', async () => {
106106
expect(handleParentKeyDown).not.toHaveBeenCalled();
107107
}
108108

109-
const allowedKeys = ['Escape', 'Tab'];
109+
// `Tab` must be checked before `Escape`: pressing `Escape` legitimately
110+
// closes the modal (a real global listener unmounts the dialog), so any
111+
// key fired on the stale `input` reference afterwards can no longer
112+
// bubble anywhere.
113+
const allowedKeys = ['Tab', 'Escape'];
110114

111115
for (const key of allowedKeys) {
112116
handleParentKeyDown.mockClear();

superset-frontend/src/features/alerts/AlertReportModal.test.tsx

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,11 @@ const generateMockedProps = (
395395
};
396396
};
397397

398+
// Matches the antd Select's rendered selection, whether it renders as
399+
// content (single/tag mode) or as a selection item (default mode).
400+
const selectedValueSelector = (title: string) =>
401+
`.ant-select-content-has-value[title="${title}"], .ant-select-selection-item[title="${title}"]`;
402+
398403
// combobox selector for mocking user input
399404
const comboboxSelect = async (
400405
element: HTMLElement,
@@ -1889,7 +1894,7 @@ test('filter reappears in dropdown after clearing with X icon', async () => {
18891894

18901895
await waitFor(() => {
18911896
const selectionItem = document.querySelector(
1892-
'.ant-select-content-has-value[title="Test Filter 1"], .ant-select-selection-item[title="Test Filter 1"]',
1897+
selectedValueSelector('Test Filter 1'),
18931898
);
18941899
expect(selectionItem).toBeInTheDocument();
18951900
});
@@ -1911,7 +1916,7 @@ test('filter reappears in dropdown after clearing with X icon', async () => {
19111916

19121917
await waitFor(() => {
19131918
const selectionItem = document.querySelector(
1914-
'.ant-select-content-has-value[title="Test Filter 1"], .ant-select-selection-item[title="Test Filter 1"]',
1919+
selectedValueSelector('Test Filter 1'),
19151920
);
19161921
expect(selectionItem).not.toBeInTheDocument();
19171922
});
@@ -2430,15 +2435,13 @@ test('edit mode shows friendly filter names instead of raw IDs', async () => {
24302435

24312436
await waitFor(() => {
24322437
const selectionItem = document.querySelector(
2433-
'.ant-select-content-has-value[title="Country"], .ant-select-selection-item[title="Country"]',
2438+
selectedValueSelector('Country'),
24342439
);
24352440
expect(selectionItem).toBeInTheDocument();
24362441
});
24372442

24382443
expect(
2439-
document.querySelector(
2440-
'.ant-select-content-has-value[title="NATIVE_FILTER-abc123"], .ant-select-selection-item[title="NATIVE_FILTER-abc123"]',
2441-
),
2444+
document.querySelector(selectedValueSelector('NATIVE_FILTER-abc123')),
24422445
).not.toBeInTheDocument();
24432446
});
24442447

@@ -2457,7 +2460,7 @@ test('edit mode falls back to raw ID when filterName is missing', async () => {
24572460

24582461
await waitFor(() => {
24592462
const selectionItem = document.querySelector(
2460-
'.ant-select-content-has-value[title="NATIVE_FILTER-xyz789"], .ant-select-selection-item[title="NATIVE_FILTER-xyz789"]',
2463+
selectedValueSelector('NATIVE_FILTER-xyz789'),
24612464
);
24622465
expect(selectionItem).toBeInTheDocument();
24632466
});
@@ -2564,9 +2567,7 @@ test('selecting filter triggers chart data request with correct params', async (
25642567

25652568
// Select the Country Filter using comboboxSelect pattern
25662569
await comboboxSelect(filterDropdown, 'Country Filter', () =>
2567-
document.querySelector(
2568-
'.ant-select-content-has-value[title="Country Filter"], .ant-select-selection-item[title="Country Filter"]',
2569-
),
2570+
document.querySelector(selectedValueSelector('Country Filter')),
25702571
);
25712572

25722573
// getChartDataRequest should have been called for filter values
@@ -2615,9 +2616,7 @@ test('selected filter excluded from other row dropdowns', async () => {
26152616

26162617
// Select Country Filter in row 1
26172618
await comboboxSelect(filterDropdown, 'Country Filter', () =>
2618-
document.querySelector(
2619-
'.ant-select-content-has-value[title="Country Filter"], .ant-select-selection-item[title="Country Filter"]',
2620-
),
2619+
document.querySelector(selectedValueSelector('Country Filter')),
26212620
);
26222621

26232622
// Wait for getChartDataRequest to complete AND state update to propagate.

0 commit comments

Comments
 (0)