github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-11-06 01:50:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Make the login dialog section links keyboard buttons (#12724)

Browse Source 
"Forgot password?", "Create an account" and "Login here." switch the
section of the login dialog (viewer and MCP authorize) and of the dialog
on the not found page. They were <a> without href, so Tab skipped them.
They become <button type="button"> with the same look; a class replaces
the bare a selector in static.scss.

A new visual test opens both dialogs, uses the keyboard to switch
sections and checks 0 px of difference at rest and on hover.

AI-assisted-by: claude-opus-5-5
This commit is contained in:
Eva Marco 2026-11-05 11:30:23 +01:00 committed by GitHub
parent 9d898b8b31
commit 657ce8b054
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 125 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

71
frontend/playwright/ui/visual-specs/visual-login.spec.js
View File

@ -37,3 +37,74 @@ test.describe("Login form", () => {
await expect(login.page).toHaveScreenshot();
});
});
test.describe("Login dialog section links", () => {
const shot = { maxDiffPixelRatio: 0, threshold: 0 };
// The links switch the dialog section: the tests use the keyboard to
// check they work as buttons.
test("Login and register modal", async ({ page }) => {
// The viewer and the MCP authorize page share this modal; the MCP page
// opens it on a 401, with no file to load.
await LoginPage.mockConfigFlags(page, ["enable-registration"]);
await page.route("**/api/mcp/oauth/request?*", (route) =>
route.fulfill({
status: 401,
contentType: "application/json",
body: "{}",
}),
);
await page.goto("/?screen=mcp-authorize&request=test");
const modal = page.getByRole("dialog");
const forgot = modal.getByText("Forgot password?", { exact: true });
await expect(forgot).toBeVisible();
await page.mouse.move(10, 10);
await expect(forgot).toHaveScreenshot(shot);
await forgot.hover();
await expect(forgot).toHaveScreenshot(shot);
const create = modal.getByText("Create an account", { exact: true });
await create.hover();
await expect(create).toHaveScreenshot(shot);
await create.focus();
await page.keyboard.press("Enter");
const loginHere = modal.getByText("Login here.", { exact: true });
await expect(loginHere).toBeVisible();
await page.mouse.move(10, 10);
await expect(loginHere).toHaveScreenshot(shot);
await loginHere.hover();
await expect(loginHere).toHaveScreenshot(shot);
await loginHere.focus();
await page.keyboard.press("Enter");
await page.mouse.move(10, 10);
await expect(modal).toHaveScreenshot(shot);
});
test("Not found page dialog", async ({ page }) => {
await LoginPage.mockConfigFlags(page, ["enable-registration"]);
// An invalid team id is a not found error; logged out users get the
// login dialog on top of it.
await page.goto("/?screen=dashboard-recent&team-id=not-a-uuid");
const create = page.getByText("Create an account", { exact: true });
await expect(create).toBeVisible();
await page.mouse.move(10, 10);
await expect(create).toHaveScreenshot(shot);
await create.hover();
await expect(create).toHaveScreenshot(shot);
await create.focus();
await page.keyboard.press("Enter");
const loginHere = page.getByText("Login here.", { exact: true });
await expect(loginHere).toBeVisible();
// The new section reuses the focused button: blur it for the rest shot.
await loginHere.blur();
await page.mouse.move(10, 10);
await expect(loginHere).toHaveScreenshot(shot);
await loginHere.hover();
await expect(loginHere).toHaveScreenshot(shot);
});
});

18
frontend/src/app/main/ui/auth/login_modal.cljs
View File

@ -102,15 +102,19 @@
:hide-recovery-link true}]
[:div {:class (stl/css :login-links)}
[:div
[:a {:on-click set-section
:data-value "recovery-request"}
[:button {:type "button"
:class (stl/css :link)
:on-click set-section
:data-value "recovery-request"}
(tr "auth.forgot-password")]]
(when (contains? cf/flags :registration)
[:div
[:span
(tr "auth.register") " "]
[:a {:on-click set-section
:data-value "register"}
[:button {:type "button"
:class (stl/css :link)
:on-click set-section
:data-value "register"}
(tr "auth.register-submit")]])]]
:register
@ -119,8 +123,10 @@
[:div {:class (stl/css :login-links)}
[:div
[:span (tr "auth.already-have-account") " "]
[:a {:on-click set-section
:data-value "login"}
[:button {:type "button"
:class (stl/css :link)
:on-click set-section
:data-value "login"}
(tr "auth.login-here")]]]]
:register-validate

18
frontend/src/app/main/ui/auth/login_modal.scss
View File

@ -72,3 +72,21 @@
position: relative;
color: var(--color-foreground-primary);
}
.link {
font: inherit;
font-weight: 500;
// A button is a block in its line: the normal line height keeps the box of
// the inline link it replaces.
line-height: normal;
color: var(--color-accent-primary);
background: none;
border: none;
padding: 0;
cursor: pointer;
&:hover {
text-decoration: underline;
}
}

13
frontend/src/app/main/ui/static.cljs
View File

@ -176,8 +176,10 @@
[:div {:class (stl/css :change-section)}
(tr "auth.register")
" "
[:a {:data-section "register"
:on-click set-section}
[:button {:type "button"
:class (stl/css :link)
:data-section "register"
:on-click set-section}
(tr "auth.register-submit")]]]
:register
@ -190,8 +192,11 @@
[:div {:class (stl/css :change-section)}
(tr "auth.already-have-account")
" "
[:a {:data-section "login"
:on-click set-section} (tr "auth.login-here")]]
[:button {:type "button"
:class (stl/css :link)
:data-section "login"
:on-click set-section}
(tr "auth.login-here")]]
[:div {:class (stl/css :links)}
[:hr {:class (stl/css :separator)}]
[:> register/terms-service-privacy-policy*]]]

17
frontend/src/app/main/ui/static.scss
View File

@ -373,9 +373,22 @@
inline-size: 100%;
text-align: center;
// Native <a> inside change-section — no class available
a {
.link {
font: inherit;
font-weight: 500;
// A button is a block in its line: the normal line height keeps the box of
// the inline link it replaces.
line-height: normal;
color: var(--color-accent-primary);
background: none;
border: none;
padding: 0;
cursor: pointer;
&:hover {
text-decoration: underline;
}
}
}