Make BaseDialog's div keyboard focusable and fix test. (#30631)

* Make BaseDialog's div keyboard focusable and fix test. * Less weird test * Update snapshots * More snapshots
2025-08-27 13:41:39 +01:00
parent c6f47cfd8e
commit 87b4918d34
26 changed files with 54 additions and 1 deletions
--- a/src/components/views/dialogs/BaseDialog.tsx
+++ b/src/components/views/dialogs/BaseDialog.tsx
@@ -145,6 +145,9 @@ export default class BaseDialog extends React.Component<IProps> {
        const lockProps: Record<string, any> = {
            "onKeyDown": this.onKeyDown,
            "role": "dialog",
+            // Allow the dialog to be keyboard focusable
+            // So the escape key handling works in more cases (say you select the header)
+            "tabIndex": -1,
            // This should point to a node describing the dialog.
            // If we were about to completely follow this recommendation we'd need to
            // make all the components relying on BaseDialog to be aware of it.
--- a/test/unit-tests/snapshots/Terms-test.tsx.snap
+++ b/test/unit-tests/snapshots/Terms-test.tsx.snap
@@ -7,6 +7,7 @@ exports[`dialogTermsInteractionCallback should render a dialog with the expected
  class=""
  data-focus-lock-disabled="false"
  role="dialog"
+  tabindex="-1"
 >
  <div
    class="mx_Dialog_header"
--- a/test/unit-tests/async-components/dialogs/security/snapshots/NewRecoveryMethodDialog-test.tsx.snap
+++ b/test/unit-tests/async-components/dialogs/security/snapshots/NewRecoveryMethodDialog-test.tsx.snap
@@ -12,6 +12,7 @@ exports[`<NewRecoveryMethodDialog /> when key backup is disabled 1`] = `
    class="mx_KeyBackupFailedDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
@@ -83,6 +84,7 @@ exports[`<NewRecoveryMethodDialog /> when key backup is enabled 1`] = `
    class="mx_KeyBackupFailedDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
--- a/test/unit-tests/components/structures/snapshots/MatrixChat-test.tsx.snap
+++ b/test/unit-tests/components/structures/snapshots/MatrixChat-test.tsx.snap
@@ -388,6 +388,7 @@ exports[`<MatrixChat /> with an existing session onAction() room actions leave_r
  class="mx_QuestionDialog mx_Dialog_fixedWidth"
  data-focus-lock-disabled="false"
  role="dialog"
+  tabindex="-1"
 >
  <div
    class="mx_Dialog_header"
@@ -444,6 +445,7 @@ exports[`<MatrixChat /> with an existing session onAction() room actions leave_r
  class="mx_QuestionDialog mx_Dialog_fixedWidth"
  data-focus-lock-disabled="false"
  role="dialog"
+  tabindex="-1"
 >
  <div
    class="mx_Dialog_header"
--- a/test/unit-tests/components/views/dialogs/BaseDialog-test.tsx
+++ b/test/unit-tests/components/views/dialogs/BaseDialog-test.tsx
@@ -14,7 +14,12 @@ import BaseDialog from "../../../../../src/components/views/dialogs/BaseDialog.t
 describe("BaseDialog", () => {
    it("calls onFinished when Escape is pressed", async () => {
        const onFinished = jest.fn();
-        render(<BaseDialog onFinished={onFinished} />);
+        const { container } = render(<BaseDialog onFinished={onFinished} />);
+        // Autolock's autofocus in the empty dialog is focusing on the close button and bringing up the tooltip
+        // So we either need to call escape twice(one for the tooltip and one for the dialog) or focus
+        // on the dialog first.
+        const dialog = container.querySelector('[role="dialog"]') as HTMLElement;
+        dialog?.focus();
        await userEvent.keyboard("{Escape}");
        expect(onFinished).toHaveBeenCalled();
    });
--- a/test/unit-tests/components/views/dialogs/snapshots/ChangelogDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/dialogs/snapshots/ChangelogDialog-test.tsx.snap
@@ -13,6 +13,7 @@ exports[`<ChangelogDialog /> should fetch github proxy url for each repo with ol
    class="mx_QuestionDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
--- a/test/unit-tests/components/views/dialogs/snapshots/ConfirmRejectInviteDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/dialogs/snapshots/ConfirmRejectInviteDialog-test.tsx.snap
@@ -13,6 +13,7 @@ exports[`ConfirmRejectInviteDialog can reject with options selected 1`] = `
    class="mx_DeclineAndBlockInviteDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
--- a/test/unit-tests/components/views/dialogs/snapshots/ConfirmUserActionDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/dialogs/snapshots/ConfirmUserActionDialog-test.tsx.snap
@@ -13,6 +13,7 @@ exports[`ConfirmUserActionDialog renders 1`] = `
    class="mx_ConfirmUserActionDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
--- a/test/unit-tests/components/views/dialogs/snapshots/DevtoolsDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/dialogs/snapshots/DevtoolsDialog-test.tsx.snap
@@ -12,6 +12,7 @@ exports[`DevtoolsDialog renders the devtools dialog 1`] = `
    class="mx_QuestionDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
--- a/test/unit-tests/components/views/dialogs/snapshots/ExportDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/dialogs/snapshots/ExportDialog-test.tsx.snap
@@ -7,6 +7,7 @@ exports[`<ExportDialog /> renders export dialog 1`] = `
  class="mx_ExportDialog false mx_Dialog_fixedWidth"
  data-focus-lock-disabled="false"
  role="dialog"
+  tabindex="-1"
 >
  <div
    class="mx_Dialog_header"
--- a/test/unit-tests/components/views/dialogs/snapshots/FeedbackDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/dialogs/snapshots/FeedbackDialog-test.tsx.snap
@@ -13,6 +13,7 @@ exports[`FeedbackDialog should respect feedback config 1`] = `
    class="mx_QuestionDialog mx_FeedbackDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
--- a/test/unit-tests/components/views/dialogs/snapshots/LogoutDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/dialogs/snapshots/LogoutDialog-test.tsx.snap
@@ -13,6 +13,7 @@ exports[`LogoutDialog Prompts user to go to settings if there is a backup on the
    class="mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
@@ -100,6 +101,7 @@ exports[`LogoutDialog Prompts user to go to settings if there is no backup on th
    class="mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
@@ -187,6 +189,7 @@ exports[`LogoutDialog shows a regular dialog when crypto is disabled 1`] = `
    class="mx_QuestionDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
--- a/test/unit-tests/components/views/dialogs/snapshots/ManageRestrictedJoinRuleDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/dialogs/snapshots/ManageRestrictedJoinRuleDialog-test.tsx.snap
@@ -12,6 +12,7 @@ exports[`<ManageRestrictedJoinRuleDialog /> should list spaces which are not par
    class="mx_ManageRestrictedJoinRuleDialog"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
@@ -189,6 +190,7 @@ exports[`<ManageRestrictedJoinRuleDialog /> should render empty state 1`] = `
    class="mx_ManageRestrictedJoinRuleDialog"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
--- a/test/unit-tests/components/views/dialogs/snapshots/ManualDeviceKeyVerificationDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/dialogs/snapshots/ManualDeviceKeyVerificationDialog-test.tsx.snap
@@ -13,6 +13,7 @@ exports[`ManualDeviceKeyVerificationDialog should render correctly 1`] = `
    class="mx_QuestionDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
--- a/test/unit-tests/components/views/dialogs/snapshots/MessageEditHistoryDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/dialogs/snapshots/MessageEditHistoryDialog-test.tsx.snap
@@ -12,6 +12,7 @@ exports[`<MessageEditHistory /> should match the snapshot 1`] = `
    class="mx_MessageEditHistoryDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
@@ -128,6 +129,7 @@ exports[`<MessageEditHistory /> should support events with 1`] = `
    class="mx_MessageEditHistoryDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
--- a/test/unit-tests/components/views/dialogs/snapshots/ReportRoomDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/dialogs/snapshots/ReportRoomDialog-test.tsx.snap
@@ -13,6 +13,7 @@ exports[`ReportRoomDialog displays admin message 1`] = `
    class="mx_ReportRoomDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
--- a/test/unit-tests/components/views/dialogs/snapshots/ServerPickerDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/dialogs/snapshots/ServerPickerDialog-test.tsx.snap
@@ -13,6 +13,7 @@ exports[`<ServerPickerDialog /> should render dialog 1`] = `
    class="mx_ServerPickerDialog"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
--- a/test/unit-tests/components/views/dialogs/snapshots/ShareDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/dialogs/snapshots/ShareDialog-test.tsx.snap
@@ -13,6 +13,7 @@ exports[`ShareDialog should not render the QR code if disabled 1`] = `
    class="mx_ShareDialog"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
@@ -143,6 +144,7 @@ exports[`ShareDialog should not render the socials if disabled 1`] = `
    class="mx_ShareDialog"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
@@ -229,6 +231,7 @@ exports[`ShareDialog should render a share dialog for a matrix event 1`] = `
    class="mx_ShareDialog"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
@@ -402,6 +405,7 @@ exports[`ShareDialog should render a share dialog for a room 1`] = `
    class="mx_ShareDialog"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
@@ -574,6 +578,7 @@ exports[`ShareDialog should render a share dialog for a room member 1`] = `
    class="mx_ShareDialog"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
@@ -719,6 +724,7 @@ exports[`ShareDialog should render a share dialog for an URL 1`] = `
    class="mx_ShareDialog"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
--- a/test/unit-tests/components/views/dialogs/snapshots/UnpinAllDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/dialogs/snapshots/UnpinAllDialog-test.tsx.snap
@@ -12,6 +12,7 @@ exports[`<UnpinAllDialog /> should render 1`] = `
    class="mx_UnpinAllDialog"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
--- a/test/unit-tests/components/views/dialogs/snapshots/UntrustedDeviceDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/dialogs/snapshots/UntrustedDeviceDialog-test.tsx.snap
@@ -12,6 +12,7 @@ exports[`<UntrustedDeviceDialog /> should display the dialog for the device of a
    class="mx_UntrustedDeviceDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
@@ -91,6 +92,7 @@ exports[`<UntrustedDeviceDialog /> should display the dialog for the device of t
    class="mx_UntrustedDeviceDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
--- a/test/unit-tests/components/views/dialogs/security/snapshots/CreateSecretStorageDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/dialogs/security/snapshots/CreateSecretStorageDialog-test.tsx.snap
@@ -12,6 +12,7 @@ exports[`CreateSecretStorageDialog handles the happy path 1`] = `
    class="mx_CreateSecretStorageDialog"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
@@ -140,6 +141,7 @@ exports[`CreateSecretStorageDialog handles the happy path 2`] = `
    class="mx_CreateSecretStorageDialog"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
@@ -231,6 +233,7 @@ exports[`CreateSecretStorageDialog when there is an error fetching the backup ve
    class="mx_CreateSecretStorageDialog"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
--- a/test/unit-tests/components/views/dialogs/security/snapshots/ExportE2eKeysDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/dialogs/security/snapshots/ExportE2eKeysDialog-test.tsx.snap
@@ -12,6 +12,7 @@ exports[`ExportE2eKeysDialog renders 1`] = `
    class="mx_exportE2eKeysDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
--- a/test/unit-tests/components/views/dialogs/security/snapshots/ImportE2eKeysDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/dialogs/security/snapshots/ImportE2eKeysDialog-test.tsx.snap
@@ -12,6 +12,7 @@ exports[`ImportE2eKeysDialog renders 1`] = `
    class="mx_importE2eKeysDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
--- a/test/unit-tests/components/views/dialogs/security/snapshots/RestoreKeyBackupDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/dialogs/security/snapshots/RestoreKeyBackupDialog-test.tsx.snap
@@ -12,6 +12,7 @@ exports[`<RestoreKeyBackupDialog /> should display an error when recovery key is
    class="mx_RestoreKeyBackupDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
@@ -112,6 +113,7 @@ exports[`<RestoreKeyBackupDialog /> should not raise an error when recovery is v
    class="mx_RestoreKeyBackupDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
@@ -211,6 +213,7 @@ exports[`<RestoreKeyBackupDialog /> should render 1`] = `
    class="mx_RestoreKeyBackupDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
@@ -309,6 +312,7 @@ exports[`<RestoreKeyBackupDialog /> should restore key backup when Recovery key
    class="mx_RestoreKeyBackupDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
@@ -374,6 +378,7 @@ exports[`<RestoreKeyBackupDialog /> should restore key backup when passphrase is
    class="mx_RestoreKeyBackupDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
@@ -439,6 +444,7 @@ exports[`<RestoreKeyBackupDialog /> should restore key backup when the key is ca
    class="mx_RestoreKeyBackupDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
@@ -504,6 +510,7 @@ exports[`<RestoreKeyBackupDialog /> should restore key backup when the key is in
    class="mx_RestoreKeyBackupDialog mx_Dialog_fixedWidth"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"
--- a/test/unit-tests/components/views/settings/tabs/room/snapshots/SecurityRoomSettingsTab-test.tsx.snap
+++ b/test/unit-tests/components/views/settings/tabs/room/snapshots/SecurityRoomSettingsTab-test.tsx.snap
@@ -117,6 +117,7 @@ exports[`<SecurityRoomSettingsTab /> join rule handles error when updating join
  class="mx_ErrorDialog mx_Dialog_fixedWidth"
  data-focus-lock-disabled="false"
  role="dialog"
+  tabindex="-1"
 >
  <div
    class="mx_Dialog_header"
@@ -159,6 +160,7 @@ exports[`<SecurityRoomSettingsTab /> join rule warns when trying to make an encr
  class="mx_QuestionDialog mx_Dialog_fixedWidth"
  data-focus-lock-disabled="false"
  role="dialog"
+  tabindex="-1"
 >
  <div
    class="mx_Dialog_header"
--- a/test/unit-tests/components/views/spaces/snapshots/AddExistingToSpaceDialog-test.tsx.snap
+++ b/test/unit-tests/components/views/spaces/snapshots/AddExistingToSpaceDialog-test.tsx.snap
@@ -13,6 +13,7 @@ exports[`<AddExistingToSpaceDialog /> looks as expected 1`] = `
    class="mx_AddExistingToSpaceDialog"
    data-focus-lock-disabled="false"
    role="dialog"
+    tabindex="-1"
  >
    <div
      class="mx_Dialog_header"