From 3b36d8df0e4936fe97cce6612472e77d7068d433 Mon Sep 17 00:00:00 2001 From: Balsa Asanovic Date: Thu, 28 Mar 2024 00:44:20 +0100 Subject: [PATCH 01/12] added keyboard support when navigating suggested usernames --- web/src/components/users/FirstUser.jsx | 36 ++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/web/src/components/users/FirstUser.jsx b/web/src/components/users/FirstUser.jsx index 43138b9c92..166b91f49b 100644 --- a/web/src/components/users/FirstUser.jsx +++ b/web/src/components/users/FirstUser.jsx @@ -82,7 +82,7 @@ const UserData = ({ user, actions }) => { ); }; -const UsernameSuggestions = ({ entries, onSelect, setInsideDropDown }) => { +const UsernameSuggestions = ({ entries, onSelect, setInsideDropDown, focusedIndex = -1 }) => { return ( { onSelect(suggestion)} > { /* TRANSLATORS: dropdown username suggestions */} @@ -131,6 +132,8 @@ export default function FirstUser() { const [isSettingPassword, setIsSettingPassword] = useState(false); const [showSuggestions, setShowSuggestions] = useState(false); const [insideDropDown, setInsideDropDown] = useState(false); + const [focusedIndex, setFocusedIndex] = useState(-1); + const [suggestions, setSuggestions] = useState([]); useEffect(() => { cancellablePromise(client.users.getUser()).then(userValues => { @@ -220,11 +223,38 @@ export default function FirstUser() { const submitDisable = formValues.userName === "" || (isSettingPassword && !usingValidPassword); const displaySuggestions = !formValues.userName && formValues.fullName && showSuggestions; + useEffect(() => { + if (displaySuggestions) { + setFocusedIndex(-1); + setSuggestions(suggestUsernames(formValues.fullName)); + } + }, [displaySuggestions, formValues.fullName]); + const onSuggestionSelected = (suggestion) => { setInsideDropDown(false); setFormValues({ ...formValues, userName: suggestion }); }; + const handleKeyDown = (event) => { + switch (event.key) { + case 'ArrowDown': + event.preventDefault(); // Prevent page scrolling + setFocusedIndex((prevIndex) => (prevIndex + 1) % suggestions.length); + break; + case 'ArrowUp': + event.preventDefault(); // Prevent page scrolling + setFocusedIndex((prevIndex) => (prevIndex - 1 + suggestions.length) % suggestions.length); + break; + case 'Enter': + if (focusedIndex >= 0) { + onSuggestionSelected(suggestions[focusedIndex]); + } + break; + default: + break; + } + }; + if (isLoading) return ; return ( @@ -266,14 +296,16 @@ export default function FirstUser() { label={_("Username")} isRequired onChange={handleInputChange} + onKeyDown={handleKeyDown} /> } /> From b5d08696c4b23dbc12eaf33a55aaa974ed8d7780 Mon Sep 17 00:00:00 2001 From: Balsa Asanovic Date: Mon, 8 Apr 2024 23:53:32 +0200 Subject: [PATCH 02/12] when starting to scroll using arrow up, it started from one before last This fixes the issue, as it doesn't substract -1 when starting with arrow up --- web/src/components/users/FirstUser.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/users/FirstUser.jsx b/web/src/components/users/FirstUser.jsx index 166b91f49b..93cd3f583a 100644 --- a/web/src/components/users/FirstUser.jsx +++ b/web/src/components/users/FirstUser.jsx @@ -243,7 +243,7 @@ export default function FirstUser() { break; case 'ArrowUp': event.preventDefault(); // Prevent page scrolling - setFocusedIndex((prevIndex) => (prevIndex - 1 + suggestions.length) % suggestions.length); + setFocusedIndex((prevIndex) => (prevIndex - (prevIndex === -1 ? 0 : 1) + suggestions.length) % suggestions.length); break; case 'Enter': if (focusedIndex >= 0) { From 25c8063f55c6bb35569f819185bebb67a8cab776 Mon Sep 17 00:00:00 2001 From: Balsa Asanovic Date: Tue, 9 Apr 2024 00:17:20 +0200 Subject: [PATCH 03/12] In case of empty suggestions array, blank dropdown was shown --- web/src/components/users/FirstUser.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/users/FirstUser.jsx b/web/src/components/users/FirstUser.jsx index 93cd3f583a..6a4fe33851 100644 --- a/web/src/components/users/FirstUser.jsx +++ b/web/src/components/users/FirstUser.jsx @@ -299,7 +299,7 @@ export default function FirstUser() { onKeyDown={handleKeyDown} /> 0} then={ Date: Tue, 9 Apr 2024 00:27:32 +0200 Subject: [PATCH 04/12] Closing dropdown on Escape button press --- web/src/components/users/FirstUser.jsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/web/src/components/users/FirstUser.jsx b/web/src/components/users/FirstUser.jsx index 6a4fe33851..3dc028d093 100644 --- a/web/src/components/users/FirstUser.jsx +++ b/web/src/components/users/FirstUser.jsx @@ -239,10 +239,12 @@ export default function FirstUser() { switch (event.key) { case 'ArrowDown': event.preventDefault(); // Prevent page scrolling + if (suggestions.length > 0) setShowSuggestions(true); setFocusedIndex((prevIndex) => (prevIndex + 1) % suggestions.length); break; case 'ArrowUp': event.preventDefault(); // Prevent page scrolling + if (suggestions.length > 0) setShowSuggestions(true); setFocusedIndex((prevIndex) => (prevIndex - (prevIndex === -1 ? 0 : 1) + suggestions.length) % suggestions.length); break; case 'Enter': @@ -250,6 +252,9 @@ export default function FirstUser() { onSuggestionSelected(suggestions[focusedIndex]); } break; + case 'Escape': + setShowSuggestions(false); + break; default: break; } From 00b6c5368b0fad4898737d2bc18af8357acf3c3e Mon Sep 17 00:00:00 2001 From: Balsa Asanovic Date: Tue, 9 Apr 2024 00:39:07 +0200 Subject: [PATCH 05/12] Hiding suggestions on Tab press fixes unexpected behavior #1124 --- web/src/components/users/FirstUser.jsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/web/src/components/users/FirstUser.jsx b/web/src/components/users/FirstUser.jsx index 3dc028d093..ab7dbf47f3 100644 --- a/web/src/components/users/FirstUser.jsx +++ b/web/src/components/users/FirstUser.jsx @@ -255,6 +255,9 @@ export default function FirstUser() { case 'Escape': setShowSuggestions(false); break; + case 'Tab': + setShowSuggestions(false); + break; default: break; } From 1a99b2cb772b345e4907967a1bae1d3b7c41880d Mon Sep 17 00:00:00 2001 From: Balsa Asanovic Date: Tue, 9 Apr 2024 00:43:40 +0200 Subject: [PATCH 06/12] Code simplification. The same action for two cases in a switch --- web/src/components/users/FirstUser.jsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/web/src/components/users/FirstUser.jsx b/web/src/components/users/FirstUser.jsx index ab7dbf47f3..6443256582 100644 --- a/web/src/components/users/FirstUser.jsx +++ b/web/src/components/users/FirstUser.jsx @@ -253,8 +253,6 @@ export default function FirstUser() { } break; case 'Escape': - setShowSuggestions(false); - break; case 'Tab': setShowSuggestions(false); break; From 5170908e13aba32ef987c511ece8ceceeffb536a Mon Sep 17 00:00:00 2001 From: balsa-asanovic Date: Wed, 17 Apr 2024 01:14:04 +0200 Subject: [PATCH 07/12] added some unit test for usernames suggestions --- web/src/components/users/FirstUser.test.jsx | 41 +++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/web/src/components/users/FirstUser.test.jsx b/web/src/components/users/FirstUser.test.jsx index 2d9b40fd03..9d4ebbe45b 100644 --- a/web/src/components/users/FirstUser.test.jsx +++ b/web/src/components/users/FirstUser.test.jsx @@ -276,3 +276,44 @@ describe("when the user has been modified", () => { screen.getByText("ytm"); }); }); + +describe("dropdown suggestions when full name is given", () => { + it("dropdown suggestions is shown on username focus", async () => { + const { user } = installerRender(); + await screen.findByText("No user defined yet."); + const button = await screen.findByRole("button", { name: "Define a user now" }); + await user.click(button); + + const dialog = await screen.findByRole("dialog"); + + const fullNameInput = within(dialog).getByLabelText("Full name"); + await user.type(fullNameInput, "Jane Doe"); + + await user.tab(); + + const menuItems = screen.getAllByText("Use suggested username"); + expect(menuItems.length).toBe(4); + }); + + it("if focused is removed, dropdown is closed", async () => { + const { user } = installerRender(); + await screen.findByText("No user defined yet."); + const button = await screen.findByRole("button", { name: "Define a user now" }); + await user.click(button); + + const dialog = await screen.findByRole("dialog"); + + const fullNameInput = within(dialog).getByLabelText("Full name"); + await user.type(fullNameInput, "Jane Doe"); + + await user.tab(); + + let menuItems = screen.getAllByText("Use suggested username"); + expect(menuItems.length).toBe(4); + + await user.tab(); + + menuItems = screen.queryAllByText("Use suggested username"); + expect(menuItems.length).toBe(0); + }); +}); From 70b7f74bfea667b2748897290323b0c18c1a49eb Mon Sep 17 00:00:00 2001 From: Balsa Asanovic Date: Thu, 18 Apr 2024 23:38:19 +0200 Subject: [PATCH 08/12] added more tests for username suggestions functionality --- web/src/components/users/FirstUser.test.jsx | 100 +++++++++++++++++--- 1 file changed, 85 insertions(+), 15 deletions(-) diff --git a/web/src/components/users/FirstUser.test.jsx b/web/src/components/users/FirstUser.test.jsx index 9d4ebbe45b..e6643e52d8 100644 --- a/web/src/components/users/FirstUser.test.jsx +++ b/web/src/components/users/FirstUser.test.jsx @@ -41,6 +41,16 @@ let setUserFn = jest.fn().mockResolvedValue(setUserResult); const removeUserFn = jest.fn(); let onUsersChangeFn = jest.fn(); +const openUserForm = async () => { + const { user } = installerRender(); + await screen.findByText("No user defined yet."); + const button = await screen.findByRole("button", { name: "Define a user now" }); + await user.click(button); + const dialog = await screen.findByRole("dialog"); + + return { user, dialog } +} + beforeEach(() => { user = emptyUser; createClient.mockImplementation(() => { @@ -277,14 +287,9 @@ describe("when the user has been modified", () => { }); }); -describe("dropdown suggestions when full name is given", () => { - it("dropdown suggestions is shown on username focus", async () => { - const { user } = installerRender(); - await screen.findByText("No user defined yet."); - const button = await screen.findByRole("button", { name: "Define a user now" }); - await user.click(button); - - const dialog = await screen.findByRole("dialog"); +describe("username suggestions", () => { + it("shows suggestions when full name is given and username gets focus", async () => { + const { user, dialog } = await openUserForm(); const fullNameInput = within(dialog).getByLabelText("Full name"); await user.type(fullNameInput, "Jane Doe"); @@ -295,13 +300,8 @@ describe("dropdown suggestions when full name is given", () => { expect(menuItems.length).toBe(4); }); - it("if focused is removed, dropdown is closed", async () => { - const { user } = installerRender(); - await screen.findByText("No user defined yet."); - const button = await screen.findByRole("button", { name: "Define a user now" }); - await user.click(button); - - const dialog = await screen.findByRole("dialog"); + it("hides suggestions when username loses focus", async () => { + const { user, dialog } = await openUserForm(); const fullNameInput = within(dialog).getByLabelText("Full name"); await user.type(fullNameInput, "Jane Doe"); @@ -316,4 +316,74 @@ describe("dropdown suggestions when full name is given", () => { menuItems = screen.queryAllByText("Use suggested username"); expect(menuItems.length).toBe(0); }); + + it("does not show suggestions when full name is not given", async () => { + const { user, dialog } = await openUserForm(); + + const fullNameInput = within(dialog).getByLabelText("Full name"); + fullNameInput.focus(); + + await user.tab(); + + const menuItems = screen.queryAllByText("Use suggested username"); + expect(menuItems.length).toBe(0); + }); + + it("hides suggestions if user types something", async () => { + const { user, dialog } = await openUserForm(); + + const fullNameInput = within(dialog).getByLabelText("Full name"); + await user.type(fullNameInput, "Jane Doe"); + + await user.tab(); + + // confirming that we have suggestions + let menuItems = screen.queryAllByText("Use suggested username"); + expect(menuItems.length).toBe(4) + + const usernameInput = within(dialog).getByLabelText("Username"); + // the user now types something + await user.type(usernameInput, "John Smith"); + + // checking if suggestions are gone + menuItems = screen.queryAllByText("Use suggested username"); + expect(menuItems.length).toBe(0) + }); + + it("fills username input with chosen suggestion", async () => { + const { user, dialog } = await openUserForm(); + + const fullNameInput = within(dialog).getByLabelText("Full name"); + await user.type(fullNameInput, "Jane Doe"); + + await user.tab(); + + const menuItem = screen.getByText('janed'); + const usernameInput = within(dialog).getByLabelText("Username"); + + await act(async () => { + menuItem.click(); + }); + + expect(usernameInput.value).toBe("janed"); + }); + + it("fills username input with chosen suggestion using keyboard for selection", async () => { + const { user, dialog } = await openUserForm(); + + const fullNameInput = within(dialog).getByLabelText("Full name"); + await user.type(fullNameInput, "Jane Doe"); + + await user.tab(); + + const menuItems = screen.getAllByRole("menuitem"); + const menuItemTwo = menuItems[1].textContent.replace("Use suggested username ", ""); + + await user.keyboard("{ArrowDown}"); + await user.keyboard("{ArrowDown}"); + await user.keyboard("{Enter}"); + + const usernameInput = within(dialog).getByLabelText("Username"); + expect(usernameInput.value).toBe(menuItemTwo); + }); }); From ff1cdb952b3db4be4869015966d101a1a76b6aa0 Mon Sep 17 00:00:00 2001 From: Balsa Asanovic Date: Wed, 1 May 2024 00:00:54 +0200 Subject: [PATCH 09/12] corrections to first user test cases --- web/src/components/users/FirstUser.test.jsx | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/web/src/components/users/FirstUser.test.jsx b/web/src/components/users/FirstUser.test.jsx index e6643e52d8..55b2c7dc84 100644 --- a/web/src/components/users/FirstUser.test.jsx +++ b/web/src/components/users/FirstUser.test.jsx @@ -48,8 +48,8 @@ const openUserForm = async () => { await user.click(button); const dialog = await screen.findByRole("dialog"); - return { user, dialog } -} + return { user, dialog }; +}; beforeEach(() => { user = emptyUser; @@ -339,7 +339,7 @@ describe("username suggestions", () => { // confirming that we have suggestions let menuItems = screen.queryAllByText("Use suggested username"); - expect(menuItems.length).toBe(4) + expect(menuItems.length).toBe(4); const usernameInput = within(dialog).getByLabelText("Username"); // the user now types something @@ -347,25 +347,24 @@ describe("username suggestions", () => { // checking if suggestions are gone menuItems = screen.queryAllByText("Use suggested username"); - expect(menuItems.length).toBe(0) + expect(menuItems.length).toBe(0); }); it("fills username input with chosen suggestion", async () => { const { user, dialog } = await openUserForm(); const fullNameInput = within(dialog).getByLabelText("Full name"); - await user.type(fullNameInput, "Jane Doe"); + await user.type(fullNameInput, "Will Power"); await user.tab(); - const menuItem = screen.getByText('janed'); + const menuItem = screen.getByText('willpower'); const usernameInput = within(dialog).getByLabelText("Username"); - await act(async () => { - menuItem.click(); - }); + await user.click(menuItem); - expect(usernameInput.value).toBe("janed"); + expect(usernameInput).toHaveFocus(); + expect(usernameInput.value).toBe("willpower"); }); it("fills username input with chosen suggestion using keyboard for selection", async () => { @@ -384,6 +383,7 @@ describe("username suggestions", () => { await user.keyboard("{Enter}"); const usernameInput = within(dialog).getByLabelText("Username"); + expect(usernameInput).toHaveFocus(); expect(usernameInput.value).toBe(menuItemTwo); }); }); From 1fcc14cac5066d5b79b2b2a63c662709955fb6eb Mon Sep 17 00:00:00 2001 From: Balsa Asanovic Date: Wed, 1 May 2024 00:01:19 +0200 Subject: [PATCH 10/12] focus now comes back to input after click in dropdown --- web/src/components/users/FirstUser.jsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/web/src/components/users/FirstUser.jsx b/web/src/components/users/FirstUser.jsx index 0cb752ec3a..9a7e9512fb 100644 --- a/web/src/components/users/FirstUser.jsx +++ b/web/src/components/users/FirstUser.jsx @@ -19,7 +19,7 @@ * find current contact information at www.suse.com. */ -import React, { useState, useEffect } from "react"; +import React, { useState, useEffect, useRef } from "react"; import { _ } from "~/i18n"; import { useCancellablePromise } from "~/utils"; @@ -134,6 +134,7 @@ export default function FirstUser() { const [insideDropDown, setInsideDropDown] = useState(false); const [focusedIndex, setFocusedIndex] = useState(-1); const [suggestions, setSuggestions] = useState([]); + const usernameInputRef = useRef(); useEffect(() => { cancellablePromise(client.users.getUser()).then(userValues => { @@ -233,6 +234,7 @@ export default function FirstUser() { const onSuggestionSelected = (suggestion) => { setInsideDropDown(false); setFormValues({ ...formValues, userName: suggestion }); + usernameInputRef.current?.focus(); }; const handleKeyDown = (event) => { @@ -302,6 +304,7 @@ export default function FirstUser() { id="userName" name="userName" aria-label={_("Username")} + ref={usernameInputRef} value={formValues.userName} label={_("Username")} isRequired From eed39177cd150126bc6704b17967bf4a1276efaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 2 May 2024 10:36:45 +0100 Subject: [PATCH 11/12] web: Please ESLint --- web/src/components/users/FirstUser.test.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/src/components/users/FirstUser.test.jsx b/web/src/components/users/FirstUser.test.jsx index 55b2c7dc84..025c788058 100644 --- a/web/src/components/users/FirstUser.test.jsx +++ b/web/src/components/users/FirstUser.test.jsx @@ -305,7 +305,7 @@ describe("username suggestions", () => { const fullNameInput = within(dialog).getByLabelText("Full name"); await user.type(fullNameInput, "Jane Doe"); - + await user.tab(); let menuItems = screen.getAllByText("Use suggested username"); @@ -360,7 +360,7 @@ describe("username suggestions", () => { const menuItem = screen.getByText('willpower'); const usernameInput = within(dialog).getByLabelText("Username"); - + await user.click(menuItem); expect(usernameInput).toHaveFocus(); From 3b0ab833ec2f26ecb7dd83d4602151b0b38f464c Mon Sep 17 00:00:00 2001 From: Balsa Asanovic Date: Thu, 2 May 2024 13:12:31 +0200 Subject: [PATCH 12/12] update to changes file --- web/package/cockpit-agama.changes | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/web/package/cockpit-agama.changes b/web/package/cockpit-agama.changes index 32a47769c2..8cc66aeef9 100644 --- a/web/package/cockpit-agama.changes +++ b/web/package/cockpit-agama.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Thu May 02 11:07:23 UTC 2024 - Balsa Asanovic + +- Added keyboard support for navigating dropdown + of suggested usernames. (gh#openSUSE/agama#1122). + ------------------------------------------------------------------- Thu Apr 25 15:04:05 UTC 2024 - José Iván López González