From 656b1b7885a8b5ac42632c6c4aea563d38cd63f9 Mon Sep 17 00:00:00 2001 From: eduardo aleixo Date: Wed, 3 Jan 2024 21:03:01 -0300 Subject: [PATCH] fix: opening user menu with Enter (#29) Closes https://github.com/eh-am/fightcade-gamepad-navigation/issues/16 --- CONTRIBUTING.md | 7 +++++ src/sections/sidebar.ts | 61 +++++++++++++++++------------------------ tests/sidebar.spec.ts | 6 ++-- 3 files changed, 36 insertions(+), 38 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index dab2228..eb20e31 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -53,3 +53,10 @@ ssh -L 8315:localhost:8315 deck@steamdeck ``` 4. Open `http://localhost:8315` in chrome + +## Debugging playwright +Use + +`PWDEBUG=1 npm run test:ui` + +to open Playwright Inspector diff --git a/src/sections/sidebar.ts b/src/sections/sidebar.ts index c4dec65..5ca93cb 100644 --- a/src/sections/sidebar.ts +++ b/src/sections/sidebar.ts @@ -12,11 +12,6 @@ export function initSidebar(): boolean { } console.log("initializing sidebar..."); - // // Technically there should be always an item - // if (!first) { - // return false; - // } - // Make leave lobby/mute channel items always visible addCSS(` /* Copied from the hover status */ @@ -61,8 +56,6 @@ position: relative !important; } `); - setupUserButton(); - return true; } @@ -89,9 +82,6 @@ function setupLobbyButtons(channelItemWrapper: HTMLElement): Teardown { const joinLobbyButton = channelItemWrapper.querySelector(".channelItem"); - // const joinLobbyButton = - // channelItemWrapper.querySelector(".channelItem"); - if (!muteChannelButton || !leaveChannelButton || !joinLobbyButton) { return () => {}; } @@ -136,9 +126,6 @@ function setupLobbyButtons(channelItemWrapper: HTMLElement): Teardown { } else if (keyPressed === "ArrowUp") { // Out of bounds e.preventDefault(); - // const items = getHighLevelVerticalItems(); - // const myIndex = Array.from(items).findIndex((a) => a === joinLobbyButton); - // CL.prev(items, myIndex).focus(); } else if (keyPressed === "ArrowDown") { e.preventDefault(); muteChannelButton.focus(); @@ -220,24 +207,24 @@ function setupUserButton(): Teardown { function onKeydown(el: HTMLElement, e: KeyboardEvent) { const keyPressed = e.key; - console.log("key pressed", keyPressed); + // TODO: we should only trigger a click + // then the click would handle focus automatically if (keyPressed === "Enter") { const logoutButton = document.querySelector( '[role="button"][aria-label="Logout"]' ); - const isActive = isVisible(logoutButton); // TODO: make this better - // The event listener upstream is listening for clicks on the img e.preventDefault(); - // TODO: we should only trigger a click - // then the click would handle focus automatically - // - // TODO: figure out the state - // el.querySelector(".userAvatarWrapper")?.click(); - el.click(); + findClickableUserButtonEl(el)?.click(); + if (!logoutButton) { + console.warn("could not find the logout button in the DOM"); + return; + } + const isActive = isVisible(logoutButton); + // It's open, we are now closing it if (isActive) { // Focus back on itself el.focus(); @@ -256,16 +243,6 @@ function setupUserButton(): Teardown { } } - console.log("adding listeners to buootns", buttons); - Array.from(buttons).map((el) => { - el.addEventListener("focusin", () => { - console.log(el, "is getting focus"); - }); - el.removeEventListener("focusout", () => { - console.log(el, "is losing focus"); - }); - }); - const bind = onKeydown.bind(null, el); el.addEventListener("keydown", bind); return () => { @@ -284,7 +261,7 @@ function setupSearchButtonRole() { } function setupLogoutButtonRole() { - const el = document.querySelector(`.logoutWrapper`); + const el = document.querySelector(`.logOutWrapper`); if (!el) { return; } @@ -460,8 +437,10 @@ function setupUserMenuKeydown(): Teardown { const n = list.next(order, activeElement); if (n === "OOB_END") { - userButton?.click(); - focusBackWhenTransitionEnds(userButton); + if (userButton) { + findClickableUserButtonEl(userButton)?.click(); + focusBackWhenTransitionEnds(userButton); + } } else if (n === "OOB_START") { el.focus(); } else { @@ -521,6 +500,8 @@ export function updateSidebar() { ); setupRoles(); + + teardown.push(setupUserButton()); teardown.push(setupRegularButtonsKeydown()); teardown.push(setupUserMenuKeydown()); @@ -535,7 +516,15 @@ export function updateSidebar() { // where tab rovering breaks upon a new lobby joined const first = sidebarItems[0]; sidebarItems.forEach((el) => el.setAttribute("tabIndex", "-1")); - first.setAttribute("tabIndex", "0"); + if (first) { + first.setAttribute("tabIndex", "0"); + } +} + +// The event listener upstream is listening for clicks on the img +// userButton should be .userButton +function findClickableUserButtonEl(userButton: HTMLElement) { + return userButton.querySelector(".userAvatarWrapper"); } function focusBackWhenTransitionEnds(userButton: HTMLElement | null) { diff --git a/tests/sidebar.spec.ts b/tests/sidebar.spec.ts index 8e804fd..68c8130 100644 --- a/tests/sidebar.spec.ts +++ b/tests/sidebar.spec.ts @@ -84,6 +84,8 @@ test("it opens the User Menu correctly", async ({ page }) => { // Out of bounds downwards, focus on the user button and uncollapse // Ideally we would check "User" is focused, but I couldn't make it work // TODO: make this work :( - //await page.keyboard.press("ArrowDown"); - // await expect(page.getByRole("button", { name: "Logout" })).not.toBeFocused(); + await page.keyboard.press("ArrowDown"); + + // This is super buggy, since .toBeFocused() returns true, but it says it's not visible + await expect(page.getByRole("button", { name: "Logout" })).not.toBeVisible(); });