AccessSecretStorageDialog: various fixes (#30093)

* AccessSecretStorageDialog: clear notice when input is empty

* AccessSecretStorageDialog: Simplify logic for calculating feedback

No functional changes, just simplification

* AccessSecretStorageDialog: use the right icon

Should be a ! in a circle, not an X. Also requires use of `Flex` to fix the
vertical alignment.

* AccessSecretStorageDialog: fix resizing when key is correct

* AccessSecretStorageDialog: remove confirmation on dialog close

Per discussion on https://github.com/element-hq/element-web/issues/30024, we
don't want this any more.

res/css/views/dialogs/security/_AccessSecretStorageDialog.pcss

@@ -32,7 +32,7 @@ Please see LICENSE files in the repository root for full details.
                 color: $alert;
 
                 &::before {
-                    mask-image: url("@vector-im/compound-design-tokens/icons/close.svg");
+                    mask-image: url("@vector-im/compound-design-tokens/icons/error-solid.svg");
                     background-color: $alert;
                 }
             }

src/SecurityManager.ts

@@ -19,7 +19,6 @@ import AccessSecretStorageDialog, {
     type KeyParams,
 } from "./components/views/dialogs/security/AccessSecretStorageDialog";
 import { ModuleRunner } from "./modules/ModuleRunner";
-import QuestionDialog from "./components/views/dialogs/QuestionDialog";
 import InteractiveAuthDialog from "./components/views/dialogs/InteractiveAuthDialog";
 
 // This stores the secret storage private keys in memory for the JS SDK. This is
@@ -50,17 +49,6 @@ export class AccessCancelledError extends Error {
     }
 }
 
-async function confirmToDismiss(): Promise<boolean> {
-    const [sure] = await Modal.createDialog(QuestionDialog, {
-        title: _t("encryption|cancel_entering_passphrase_title"),
-        description: _t("encryption|cancel_entering_passphrase_description"),
-        danger: false,
-        button: _t("action|go_back"),
-        cancelButton: _t("action|cancel"),
-    }).finished;
-    return !sure;
-}
-
 function makeInputToKey(
     keyInfo: SecretStorage.SecretStorageKeyDescription,
 ): (keyParams: KeyParams) => Promise<Uint8Array> {
@@ -134,17 +122,6 @@ async function getSecretStorageKey(
                 return MatrixClientPeg.safeGet().secretStorage.checkKey(key, keyInfo);
             },
         },
-        /* className= */ undefined,
-        /* isPriorityModal= */ false,
-        /* isStaticModal= */ false,
-        /* options= */ {
-            onBeforeClose: async (reason): Promise<boolean> => {
-                if (reason === "backgroundClick") {
-                    return confirmToDismiss();
-                }
-                return true;
-            },
-        },
     );
     const [keyParams] = await finished;
     if (!keyParams) {

src/components/views/dialogs/security/AccessSecretStorageDialog.tsx

@@ -14,6 +14,7 @@ import React, { type ChangeEvent, type FormEvent } from "react";
 import { type SecretStorage } from "matrix-js-sdk/src/matrix";
 
 import Field from "../../elements/Field";
+import { Flex } from "../../../utils/Flex";
 import { _t } from "../../../../languageHandler";
 import { EncryptionCard } from "../../settings/encryption/EncryptionCard";
 import { EncryptionCardButtons } from "../../settings/encryption/EncryptionCardButtons";
@@ -84,6 +85,7 @@ export default class AccessSecretStorageDialog extends React.PureComponent<IProp
             this.setState({
                 recoveryKeyCorrect: null,
             });
+            return;
         }
 
         const hasPassphrase = this.props.keyInfo?.passphrase?.salt && this.props.keyInfo?.passphrase?.iterations;
@@ -140,30 +142,30 @@ export default class AccessSecretStorageDialog extends React.PureComponent<IProp
         }
     };
 
-    private getKeyValidationClasses(): string {
-        return classNames({
-            "mx_AccessSecretStorageDialog_recoveryKeyFeedback": this.state.recoveryKeyCorrect !== null,
-            "mx_AccessSecretStorageDialog_recoveryKeyFeedback--invalid": this.state.recoveryKeyCorrect === false,
+    private getRecoveryKeyFeedback(): React.ReactNode | null {
+        let validationText: string;
+        let classes: string | undefined;
+
+        if (this.state.recoveryKeyCorrect) {
+            // The recovery key is good. Empty feedback.
+            validationText = "\xA0"; // &nbsp;
+        } else if (this.state.recoveryKeyCorrect === null) {
+            // The input element is empty. Tell the user they can also use a passphrase.
+            validationText = _t("encryption|access_secret_storage_dialog|alternatives");
+        } else {
+            // The entered key is not (yet) correct. Tell them so.
+            validationText = _t("encryption|access_secret_storage_dialog|key_validation_text|wrong_security_key");
+            classes = classNames({
+                "mx_AccessSecretStorageDialog_recoveryKeyFeedback": true,
+                "mx_AccessSecretStorageDialog_recoveryKeyFeedback--invalid": true,
             });
         }
 
-    private getKeyValidationText(): string | null {
-        if (this.state.recoveryKeyCorrect) {
-            return null;
-        } else if (this.state.recoveryKeyCorrect === null) {
-            return _t("encryption|access_secret_storage_dialog|alternatives");
-        } else {
-            return _t("encryption|access_secret_storage_dialog|key_validation_text|wrong_security_key");
-        }
-    }
-
-    private getRecoveryKeyFeedback(): React.ReactNode | null {
-        const validationText = this.getKeyValidationText();
-        if (validationText === null) {
-            return null;
-        } else {
-            return <div className={this.getKeyValidationClasses()}>{validationText}</div>;
-        }
+        return (
+            <Flex align="center" className={classes}>
+                {validationText}
+            </Flex>
+        );
     }
 
     public render(): React.ReactNode {

src/i18n/strings/en_EN.json

@@ -920,8 +920,6 @@
             "security_key_title": "Recovery key"
         },
         "bootstrap_title": "Setting up keys",
-        "cancel_entering_passphrase_description": "Are you sure you want to cancel entering passphrase?",
-        "cancel_entering_passphrase_title": "Cancel entering passphrase?",
         "confirm_encryption_setup_body": "Click the button below to confirm setting up encryption.",
         "confirm_encryption_setup_title": "Confirm encryption setup",
         "cross_signing_room_normal": "This room is end-to-end encrypted",

test/unit-tests/components/views/dialogs/AccessSecretStorageDialog-test.tsx

@@ -29,15 +29,15 @@ describe("AccessSecretStorageDialog", () => {
         render(<AccessSecretStorageDialog {...defaultProps} {...props} />);
     };
 
-    const enterRecoveryKey = (): void => {
-        act(() => {
+    const enterRecoveryKey = async (valueToEnter: string = recoveryKey): Promise<void> => {
+        await act(async () => {
             fireEvent.change(screen.getByRole("textbox"), {
                 target: {
-                    value: recoveryKey,
+                    value: valueToEnter,
                 },
             });
-            // wait for debounce
-            jest.advanceTimersByTime(250);
+            // wait for debounce, and then give `checkPrivateKey` a chance to complete
+            await jest.advanceTimersByTimeAsync(250);
         });
     };
 
@@ -116,4 +116,22 @@ describe("AccessSecretStorageDialog", () => {
         await expect(screen.findByText("The recovery key you entered is not correct.")).resolves.toBeInTheDocument();
         expect(screen.getByText("Continue")).toHaveAttribute("aria-disabled", "true");
     });
+
+    it("Clears the 'invalid recovery key' notice when the input is cleared", async function () {
+        renderComponent({ onFinished: () => {}, checkPrivateKey: () => false });
+
+        jest.spyOn(mockClient.secretStorage, "checkKey").mockRejectedValue(new Error("invalid key"));
+
+        // First, enter the wrong recovery key
+        await enterRecoveryKey();
+        expect(screen.getByText("The recovery key you entered is not correct.")).toBeInTheDocument();
+
+        // Now, clear the input: the notice should be cleared.
+        await enterRecoveryKey("");
+        expect(screen.queryByText("The recovery key you entered is not correct.")).not.toBeInTheDocument();
+        expect(
+            screen.getByText("If you have a security key or security phrase, this will work too."),
+        ).toBeInTheDocument();
+        expect(screen.getByText("Continue")).toHaveAttribute("aria-disabled", "true");
+    });
 });