Uh oh!
There was an error while loading. Please reload this page.
test(e2e): restore sidebar specs after the SideNav migration - #1871
Merged
Conversation
PR #1860 renders sidebar sessions as Astryx SideNavItem rows. SideNavItem renders endContent (the row actions MoreMenu trigger) inside the row's primary <button>, so the row button's accessible name became the composite of its contents and ends with the trigger's label. Name-based role queries then matched both the row button and the trigger and failed with strict-mode violations in three sidebar-navigation specs. The nesting is Astryx SideNavItem's own DOM — SideNavItem also drops extra props in expanded mode, so the parent's name cannot be overridden — and pulling the trigger out of the item button would need sidebar.css, frozen by in-review PR #1857. The locators are tightened instead: - project rename: scope the name query to the project's own end content (.maka-project-item-end), the only place its trigger can live. - session specs: match the trigger's name exactly (exact: true). Also moved the project disclosure-state capture ahead of entering edit mode: starting a rename swaps the whole project row for the rename input, so button[aria-expanded] is not mounted while editing (the pre-#1860 TreeList kept the row mounted). The assertion still verifies that committing a rename does not toggle the disclosure.
The 'sidebar list scrolls independently' spec pinned 60 rendered rows via [data-session-id][title^="会话 "]. Deleted the pin; two reasons: - Its only documented purpose was to prove the list overflows — the precondition the geometry assertions depend on. The spec already asserts that directly at (1a) via scrollHeight - clientHeight > 50, which is strictly stronger than a row count. - PR #1860 changed the rows' title to session metadata (formatSessionMeta), so a title-prefixed count can no longer select the rows at all; the pin had degraded to asserting 0 elements. What the pin protected (the renderer renders enough seeded rows to overflow) is now protected by the (1a) scroller-metrics poll; what it incidentally checked (60 sessions on disk) lives in the fixture unit tests (apps/desktop/src/main/__tests__/e2e-fixture.test.ts).
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Main's e2e job has been red since #1860 (52aca7b) rendered the sidebar sessions as Astryx
SideNavItemrows. Four specs failed, two root causes:Strict-mode violations in three sidebar-navigation specs.
SideNavItemrendersendContent— the row-actionsMoreMenutrigger — inside the row's primary<button>, so the row button's accessible name became the composite of its contents and ends with the trigger's label (会话 00 对话操作,maka 项目目录不可用 66 maka 项目操作). Substring name queries then matched both the row button and the trigger. The nesting isSideNavItem's own DOM (it also drops extra props in expanded mode, so the parent's name cannot be overridden), and pulling the trigger out of the item button would needapps/desktop/src/renderer/**CSS frozen by in-review PR #1857 — so the locators are tightened instead: the project spec scopes the name query to the project's own end content (.maka-project-item-end), and the session specs match the trigger name exactly (exact: true).The geometry spec's 60-row pin selected rows by
title^="会话 ", but #1860 changed the rows'titleto session metadata (formatSessionMeta), so the pin resolved to 0 elements. Its only documented purpose was proving the list overflows, which the spec already asserts directly at (1a) viascrollHeight - clientHeight > 50; it is deleted, not re-located (commit50da81d9fdetails what it protected and what protects it now).Root cause
<button>nested inside<button>is invalid HTML and a genuine a11y defect (the row button's name swallows the trigger's label). It cannot be fixed on this branch: the nesting isSideNavItem's own rendering, and moving the trigger out of the item button needs sidebar CSS that PR #1857 currently holds. Worth a follow-up upstream (AstryxendContentplacement) or after #1857 lands.One behavior note from fixing the project-rename spec: starting a rename swaps the whole project row for the rename input, so the disclosure control is unmounted while editing, and because the collapse state is uncontrolled inside the row, renaming a collapsed project would reset it to expanded after commit. The fixtures only start expanded, so no spec exercises that path; the test now captures the disclosure state while the control is mounted.
Verification
npm --workspace @maka/desktop run e2e— 70 passed / 0 failed (was 4 failed on main).npm run format:check— clean.npm run lint— clean.npm run typecheck— clean.node scripts/check-dead-css.mjs --check— no dead classes.npm --workspace @maka/desktop run test— 1305 passed.npm --workspace @maka/ui run test— 238 passed.