From 6e27c5463ddb00f5b7df58691eec7a3dce7b7bba Mon Sep 17 00:00:00 2001 From: Sebastian Date: Wed, 1 Mar 2023 15:48:39 +0100 Subject: [PATCH 1/7] remove autocomplete from Field set feedback according to validity refocus secure phrase field after validation error --- .../security/AccessSecretStorageDialog.tsx | 10 +++- src/components/views/elements/Field.tsx | 1 + src/components/views/elements/Tooltip.tsx | 4 +- .../AccessSecretStorageDialog-test.tsx | 5 ++ test/components/views/elements/Field-test.tsx | 59 ++++++++++++++++++- 5 files changed, 74 insertions(+), 5 deletions(-) diff --git a/src/components/views/dialogs/security/AccessSecretStorageDialog.tsx b/src/components/views/dialogs/security/AccessSecretStorageDialog.tsx index e6a5c67bfd8..df2a54877f4 100644 --- a/src/components/views/dialogs/security/AccessSecretStorageDialog.tsx +++ b/src/components/views/dialogs/security/AccessSecretStorageDialog.tsx @@ -63,6 +63,7 @@ interface IState { */ export default class AccessSecretStorageDialog extends React.PureComponent { private fileUpload = React.createRef(); + private inputRef = React.createRef(); public constructor(props: IProps) { super(props); @@ -178,7 +179,10 @@ export default class AccessSecretStorageDialog extends React.PureComponent | React.MouseEvent): Promise => { ev.preventDefault(); - if (this.state.passPhrase.length <= 0) return; + if (this.state.passPhrase.length <= 0) { + this.inputRef.current?.focus(); + return; + } this.setState({ keyMatches: null }); const input = { passphrase: this.state.passPhrase }; @@ -187,6 +191,7 @@ export default class AccessSecretStorageDialog extends React.PureComponent {keyStatus} diff --git a/src/components/views/elements/Field.tsx b/src/components/views/elements/Field.tsx index bcf8b00d4fa..3663fa7fe9f 100644 --- a/src/components/views/elements/Field.tsx +++ b/src/components/views/elements/Field.tsx @@ -295,6 +295,7 @@ export default class Field extends React.PureComponent { visible={(this.state.focused && forceTooltipVisible) || this.state.feedbackVisible} label={tooltipContent || this.state.feedback} alignment={Tooltip.Alignment.Right} + role={tooltipContent ? 'tooltip' : this.state.valid ? 'status' : 'alert'} /> ); } diff --git a/src/components/views/elements/Tooltip.tsx b/src/components/views/elements/Tooltip.tsx index 98ee56b1595..1015767e9b7 100644 --- a/src/components/views/elements/Tooltip.tsx +++ b/src/components/views/elements/Tooltip.tsx @@ -51,6 +51,8 @@ export interface ITooltipProps { id?: string; // If the parent is over this width, act as if it is only this wide maxParentWidth?: number; + // aria-role passed to the tooltip + role?: React.AriaRole; } type State = Partial>; @@ -186,7 +188,7 @@ export default class Tooltip extends React.PureComponent { style.display = this.props.visible ? "block" : "none"; const tooltip = ( -
+
{this.props.label}
diff --git a/test/components/views/dialogs/AccessSecretStorageDialog-test.tsx b/test/components/views/dialogs/AccessSecretStorageDialog-test.tsx index 4ce83370801..87e5647daaa 100644 --- a/test/components/views/dialogs/AccessSecretStorageDialog-test.tsx +++ b/test/components/views/dialogs/AccessSecretStorageDialog-test.tsx @@ -134,5 +134,10 @@ describe("AccessSecretStorageDialog", () => { "👎 Unable to access secret storage. Please verify that you entered the correct Security Phrase.", ), ).toBeInTheDocument(); + + + expect( + document.activeElement + ).toEqual(screen.getByPlaceholderText("Security Phrase")) }); }); diff --git a/test/components/views/elements/Field-test.tsx b/test/components/views/elements/Field-test.tsx index 199f1c25577..594005bb249 100644 --- a/test/components/views/elements/Field-test.tsx +++ b/test/components/views/elements/Field-test.tsx @@ -15,7 +15,7 @@ limitations under the License. */ import React from "react"; -import { render, screen } from "@testing-library/react"; +import { act, fireEvent, render, screen } from "@testing-library/react"; import Field from "../../../../src/components/views/elements/Field"; @@ -51,4 +51,61 @@ describe("Field", () => { expect(screen.getByRole("textbox")).not.toHaveAttribute("placeholder", "my placeholder"); }); }); + + describe("Feedback", () => { + it("Should mark the feedback as alert if invalid", async () => { + render( + Promise.resolve({ valid: false, feedback: "Invalid" })} + />, + ); + + // When invalid + await act(async () => { + fireEvent.focus(screen.getByRole("textbox")); + }); + + // Expect 'alert' role + expect(screen.queryByRole("alert")).toBeInTheDocument(); + }); + + it("Should mark the feedback as status if valid", async () => { + render( + Promise.resolve({ valid: true, feedback: "Valid" })} + />, + ); + + // When valid + await act(async () => { + fireEvent.focus(screen.getByRole("textbox")); + }); + + // Expect 'status' role + expect(screen.queryByRole("status")).toBeInTheDocument(); + }); + + it("Should mark the feedback as tooltip if custom tooltip set", async () => { + render( + Promise.resolve({ valid: true, feedback: "Valid" })} + tooltipContent={"Tooltip"} + />, + ); + + // When valid or invalid and 'tooltipContent' set + await act(async () => { + fireEvent.focus(screen.getByRole("textbox")); + }); + + // Expect 'tooltip' role + expect(screen.queryByRole("tooltip")).toBeInTheDocument(); + }); + }); }); From dd896dc5edbda8b70fde82df92a6a9467cc0c46f Mon Sep 17 00:00:00 2001 From: Sebastian Date: Tue, 7 Mar 2023 17:25:12 +0100 Subject: [PATCH 2/7] apply lint fix --- src/components/views/elements/Field.tsx | 2 +- src/components/views/elements/Tooltip.tsx | 2 +- .../views/dialogs/AccessSecretStorageDialog-test.tsx | 5 +---- test/components/views/elements/Field-test.tsx | 2 +- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/components/views/elements/Field.tsx b/src/components/views/elements/Field.tsx index 3663fa7fe9f..c12969d4382 100644 --- a/src/components/views/elements/Field.tsx +++ b/src/components/views/elements/Field.tsx @@ -295,7 +295,7 @@ export default class Field extends React.PureComponent { visible={(this.state.focused && forceTooltipVisible) || this.state.feedbackVisible} label={tooltipContent || this.state.feedback} alignment={Tooltip.Alignment.Right} - role={tooltipContent ? 'tooltip' : this.state.valid ? 'status' : 'alert'} + role={tooltipContent ? "tooltip" : this.state.valid ? "status" : "alert"} /> ); } diff --git a/src/components/views/elements/Tooltip.tsx b/src/components/views/elements/Tooltip.tsx index 1015767e9b7..4a98b0a3300 100644 --- a/src/components/views/elements/Tooltip.tsx +++ b/src/components/views/elements/Tooltip.tsx @@ -188,7 +188,7 @@ export default class Tooltip extends React.PureComponent { style.display = this.props.visible ? "block" : "none"; const tooltip = ( -
+
{this.props.label}
diff --git a/test/components/views/dialogs/AccessSecretStorageDialog-test.tsx b/test/components/views/dialogs/AccessSecretStorageDialog-test.tsx index 87e5647daaa..037cb4e3d98 100644 --- a/test/components/views/dialogs/AccessSecretStorageDialog-test.tsx +++ b/test/components/views/dialogs/AccessSecretStorageDialog-test.tsx @@ -135,9 +135,6 @@ describe("AccessSecretStorageDialog", () => { ), ).toBeInTheDocument(); - - expect( - document.activeElement - ).toEqual(screen.getByPlaceholderText("Security Phrase")) + expect(document.activeElement).toEqual(screen.getByPlaceholderText("Security Phrase")); }); }); diff --git a/test/components/views/elements/Field-test.tsx b/test/components/views/elements/Field-test.tsx index 594005bb249..ce826282aca 100644 --- a/test/components/views/elements/Field-test.tsx +++ b/test/components/views/elements/Field-test.tsx @@ -95,7 +95,7 @@ describe("Field", () => { value="" validateOnFocus onValidate={() => Promise.resolve({ valid: true, feedback: "Valid" })} - tooltipContent={"Tooltip"} + tooltipContent="Tooltip" />, ); From 67482b526e1b738dfec662285e251b0b69611345 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Wed, 8 Mar 2023 11:15:05 +0100 Subject: [PATCH 3/7] reenable 'new-password' autocomplete for secure phrase field --- .../views/dialogs/security/AccessSecretStorageDialog.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/components/views/dialogs/security/AccessSecretStorageDialog.tsx b/src/components/views/dialogs/security/AccessSecretStorageDialog.tsx index df2a54877f4..d5fc2bcede0 100644 --- a/src/components/views/dialogs/security/AccessSecretStorageDialog.tsx +++ b/src/components/views/dialogs/security/AccessSecretStorageDialog.tsx @@ -364,6 +364,7 @@ export default class AccessSecretStorageDialog extends React.PureComponent {keyStatus}
From e6538373eab034c462b631e47cf863c9a2feb58c Mon Sep 17 00:00:00 2001 From: Sebastian Date: Wed, 8 Mar 2023 11:21:17 +0100 Subject: [PATCH 4/7] - refactor Field component feedback role assignment - refactor AccessSecretStorage input focus test --- src/components/views/elements/Field.tsx | 9 ++++++++- .../views/dialogs/AccessSecretStorageDialog-test.tsx | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/components/views/elements/Field.tsx b/src/components/views/elements/Field.tsx index c12969d4382..7e253563598 100644 --- a/src/components/views/elements/Field.tsx +++ b/src/components/views/elements/Field.tsx @@ -289,13 +289,20 @@ export default class Field extends React.PureComponent { // Handle displaying feedback on validity let fieldTooltip; if (tooltipContent || this.state.feedback) { + let role; + if (tooltipContent) { + role = 'tooltip'; + } else { + role = this.state.valid ? 'status' : 'alert' + } + fieldTooltip = ( ); } diff --git a/test/components/views/dialogs/AccessSecretStorageDialog-test.tsx b/test/components/views/dialogs/AccessSecretStorageDialog-test.tsx index 037cb4e3d98..7d66f39dfb5 100644 --- a/test/components/views/dialogs/AccessSecretStorageDialog-test.tsx +++ b/test/components/views/dialogs/AccessSecretStorageDialog-test.tsx @@ -135,6 +135,6 @@ describe("AccessSecretStorageDialog", () => { ), ).toBeInTheDocument(); - expect(document.activeElement).toEqual(screen.getByPlaceholderText("Security Phrase")); + expect(screen.getByPlaceholderText("Security Phrase")).toHaveFocus(); }); }); From 95046f194027f3dd5cc62db9fb60b7b3bca01eee Mon Sep 17 00:00:00 2001 From: Sebastian Date: Wed, 8 Mar 2023 11:35:26 +0100 Subject: [PATCH 5/7] fix linting in Field component --- src/components/views/elements/Field.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/views/elements/Field.tsx b/src/components/views/elements/Field.tsx index 7e253563598..ca2393109b6 100644 --- a/src/components/views/elements/Field.tsx +++ b/src/components/views/elements/Field.tsx @@ -291,9 +291,9 @@ export default class Field extends React.PureComponent { if (tooltipContent || this.state.feedback) { let role; if (tooltipContent) { - role = 'tooltip'; + role = "tooltip"; } else { - role = this.state.valid ? 'status' : 'alert' + role = this.state.valid ? "status" : "alert"; } fieldTooltip = ( From 4f991e60f33eded135f79c4ffb44eeeb0f5e05f5 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Wed, 8 Mar 2023 11:38:04 +0100 Subject: [PATCH 6/7] reenable autocomplete 'off' for secure key --- .../views/dialogs/security/AccessSecretStorageDialog.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/dialogs/security/AccessSecretStorageDialog.tsx b/src/components/views/dialogs/security/AccessSecretStorageDialog.tsx index d5fc2bcede0..a261e441041 100644 --- a/src/components/views/dialogs/security/AccessSecretStorageDialog.tsx +++ b/src/components/views/dialogs/security/AccessSecretStorageDialog.tsx @@ -410,7 +410,7 @@ export default class AccessSecretStorageDialog extends React.PureComponent
From c0b31a593e3c966529f02cab603bfe374c7829e0 Mon Sep 17 00:00:00 2001 From: Sebbones Date: Wed, 8 Mar 2023 12:05:52 +0100 Subject: [PATCH 7/7] add typing for role in Field component Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/views/elements/Field.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/elements/Field.tsx b/src/components/views/elements/Field.tsx index ca2393109b6..2e533e19253 100644 --- a/src/components/views/elements/Field.tsx +++ b/src/components/views/elements/Field.tsx @@ -289,7 +289,7 @@ export default class Field extends React.PureComponent { // Handle displaying feedback on validity let fieldTooltip; if (tooltipContent || this.state.feedback) { - let role; + let role: React.AriaRole; if (tooltipContent) { role = "tooltip"; } else {