mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-22 16:25:58 +00:00
fix(desktop): address review — overlay a11y, e2e typecheck, nits
Blocking #1 — gateway-connecting-overlay.tsx reduced-motion regression: the top `if (reduce) setPhase('gone')` fired unconditionally on mount whenever reduce-motion was on, so every OS reduced-motion user lost the CONNECTING overlay during cold boot entirely (jumped to 'gone' before the gateway was even open). The intent was to skip the exit *choreography*, not to skip showing the overlay. Removed the unconditional top block and the redundant nested preview block; kept only the third branch (`gatewayState === 'open' && shownRef.current` → `reduce ? 'gone' : 'text-out'`) which correctly gates the short-circuit on connect. Also fixed `if(reduce)` missing-space, 6-space misindent, and the same 3-line comment pasted three times. Nit #1 — tsconfig excludes e2e, so specs were never typechecked in CI. Added tsconfig.e2e.json (extends base, includes e2e/ + playwright.config.ts, adds @playwright/test types) and wired it into the typecheck script. This surfaced three latent type errors that are fixed in the same commit: - fix-electron-tracing.ts: `app._context` and `electron._playwright` are private APIs — added `as any` on the access before the existing cast. - playwright.config.ts: `reducedMotion: 'reduce'` directly under `use:` is not a valid UseOptions property in playwright 1.58; it's a BrowserContextOption accessed via `contextOptions: { reducedMotion: 'reduce' }`. The old form was silently ignored at runtime, so reduced-motion emulation wasn't actually active — screenshots could catch overlays mid-fade (exactly what the comment warned about). Nit #2 — fix-electron-tracing.ts reaches into Playwright internals (_playwright, _allContexts, _context) with no public contract. Added a header comment calling out the `@playwright/test` exact pin (=1.58.2) so a future bump knows to re-verify the private symbols still exist. Nit #3 — main.ts TEST_WORKER_INDEX block had stray 6-space indentation. Verified: tsc -p . && tsconfig.electron && tsconfig.e2e → 0 errors; vitest boot-failure-overlay (3/3) + boot-failure-reauth (21/21) pass; npm run build clean; playwright e2e/boot-failure.spec.ts 2/2 pass.
This commit is contained in:
parent
0b40ba10cd
commit
2e10d7b942
6 changed files with 29 additions and 24 deletions
|
|
@ -21,6 +21,12 @@
|
|||
* so the test runner's willStartTest doesn't throw "already started".
|
||||
*
|
||||
* Imported from playwright.config.ts so it runs before any test.
|
||||
*
|
||||
* Pinned dependency: this file reaches into Playwright internals (_playwright,
|
||||
* _allContexts, _context) that have no public contract. @playwright/test is
|
||||
* pinned exact (=1.58.2 in package.json) so a bump can't silently break the
|
||||
* monkeypatch. When bumping, re-verify these private symbols still exist on
|
||||
* the Electron / PlaywrightInternal classes and that tracing still merges.
|
||||
*/
|
||||
|
||||
import { _electron as electron, type BrowserContext } from '@playwright/test'
|
||||
|
|
@ -31,13 +37,13 @@ const originalLaunch = electron.launch.bind(electron)
|
|||
|
||||
electron.launch = async (options: any) => {
|
||||
const app = await originalLaunch(options)
|
||||
const ctx = app._context as BrowserContext
|
||||
const ctx = (app as any)._context as BrowserContext
|
||||
electronContexts.add(ctx)
|
||||
ctx.once('close', () => electronContexts.delete(ctx))
|
||||
|
||||
// Patch _allContexts so the test runner sees the electron context
|
||||
// (didFinishTest cleanup → _stopTracing → stopChunk → merge into trace.zip).
|
||||
const pw = electron._playwright as any
|
||||
const pw = (electron as any)._playwright as any
|
||||
if (pw && !pw.__electronTracingPatched) {
|
||||
pw.__electronTracingPatched = true
|
||||
const original = pw._allContexts.bind(pw)
|
||||
|
|
|
|||
|
|
@ -7622,9 +7622,9 @@ function createWindow() {
|
|||
// Under Playright testing, instantly show the window.
|
||||
// `ready-to-show` doesn't fire in some testing envs.
|
||||
if (process.env.TEST_WORKER_INDEX !== undefined) {
|
||||
if (mainWindow && !mainWindow.isDestroyed() && !mainWindow.isVisible()) {
|
||||
mainWindow.show()
|
||||
}
|
||||
if (mainWindow && !mainWindow.isDestroyed() && !mainWindow.isVisible()) {
|
||||
mainWindow.show()
|
||||
}
|
||||
}
|
||||
|
||||
mainWindow.on('will-enter-full-screen', () => sendWindowStateChanged(true))
|
||||
|
|
|
|||
|
|
@ -41,7 +41,7 @@
|
|||
"test:desktop:nsis": "node scripts/test-desktop.mjs nsis",
|
||||
"test:desktop:existing": "node scripts/test-desktop.mjs existing",
|
||||
"test:desktop:fresh": "node scripts/test-desktop.mjs fresh",
|
||||
"typecheck": "tsc -p . --noEmit && tsc -p tsconfig.electron.json --noEmit",
|
||||
"typecheck": "tsc -p . --noEmit && tsc -p tsconfig.electron.json --noEmit && tsc -p tsconfig.e2e.json --noEmit",
|
||||
"lint": "eslint src/ electron/",
|
||||
"lint:fix": "eslint src/ electron/ --fix",
|
||||
"fmt": "prettier --write 'src/**/*.{ts,tsx}' 'electron/**/*.ts' 'vite.config.ts'",
|
||||
|
|
|
|||
|
|
@ -45,7 +45,9 @@ export default defineConfig({
|
|||
// overlay, DecodeText). Without this, screenshots capture the loading bar
|
||||
// or overlay at a transient opacity because the text-content check fires
|
||||
// before the visual transition finishes.
|
||||
reducedMotion: 'reduce',
|
||||
contextOptions: {
|
||||
reducedMotion: 'reduce',
|
||||
},
|
||||
},
|
||||
expect: {
|
||||
toHaveScreenshot: {
|
||||
|
|
|
|||
|
|
@ -83,29 +83,17 @@ export function GatewayConnectingOverlay() {
|
|||
return
|
||||
}
|
||||
|
||||
if(reduce) {
|
||||
// Under reduced motion, skip straight to gone — no text-out, no hold,
|
||||
// no overlay fade. The overlay unmounts immediately.
|
||||
setPhase('gone')
|
||||
}
|
||||
|
||||
if (previewing) {
|
||||
// Under reduced motion, skip straight to gone — no text-out, no hold,
|
||||
// no overlay fade. The overlay unmounts immediately.
|
||||
if(reduce) {
|
||||
setPhase('gone')
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
if (previewing) {
|
||||
const id = window.setTimeout(() => setPhase('text-out'), PREVIEW_CONNECT_MS)
|
||||
|
||||
return () => window.clearTimeout(id)
|
||||
}
|
||||
|
||||
if (gatewayState === 'open' && shownRef.current) {
|
||||
// Under reduced motion, skip straight to gone — no text-out, no hold,
|
||||
// no overlay fade. The overlay unmounts immediately.
|
||||
// Under reduced motion, skip the multi-phase exit choreography
|
||||
// (text-out → hold → overlay fade) and jump straight to gone so the
|
||||
// overlay unmounts the instant the gateway opens. E2E screenshots
|
||||
// rely on this to avoid catching the overlay mid-fade.
|
||||
setPhase(reduce ? 'gone' : 'text-out')
|
||||
}
|
||||
}, [phase, previewing, gatewayState, reduce])
|
||||
|
|
|
|||
9
apps/desktop/tsconfig.e2e.json
Normal file
9
apps/desktop/tsconfig.e2e.json
Normal file
|
|
@ -0,0 +1,9 @@
|
|||
{
|
||||
"extends": "./tsconfig.json",
|
||||
"compilerOptions": {
|
||||
"types": ["node", "@playwright/test"],
|
||||
"composite": true
|
||||
},
|
||||
"include": ["e2e", "playwright.config.ts"],
|
||||
"exclude": ["src", "electron"]
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue