fix(status-bar): make status title tap reliably open rename on iOS#31
Open
goyamegh wants to merge 5 commits into
Open
fix(status-bar): make status title tap reliably open rename on iOS#31goyamegh wants to merge 5 commits into
goyamegh wants to merge 5 commits into
Conversation
On iOS Safari, tapping the status-title <span> in the top header was interpreted as native text-selection (showing the blue selection handles + magnifier) instead of firing the click handler that swaps the span for a rename <input>. As a result the rename input never appeared and the session could not be renamed from the header on iPhone. Mirror what we already do on .sessionItemMarkerBtn: mark the title as non-user-selectable, suppress the iOS callout, and set touch-action: manipulation so taps go straight to the click handler without the 300ms delay. The element is already role="button" and keyboard-activatable, so disabling text selection only removes a behaviour that was incompatible with the button contract. Tests: - e2e: assert the rendered .statusTitle has user-select: none and touch-action: manipulation (regression for the CSS contract). - e2e: on the mobile project, simulate a real touchscreen tap and assert the rename input becomes visible and focused, then rename the session through it. Signed-off-by: Megha Goyal <goyamegh@amazon.com>
There was a problem hiding this comment.
Pull request overview
Fixes an iOS Safari usability bug where tapping the status/session title in the header triggers native text selection instead of the rename interaction, preventing inline renaming from opening.
Changes:
- Update
.statusTitlestyling to disable text selection/callout on iOS and prefer tap-style interactions (touch-action: manipulation). - Add Playwright e2e coverage for the CSS “non-selectable” contract and for a real touch tap flow on the mobile project.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/styles/statusBar.css |
Adds iOS/touch-focused CSS to ensure taps on the status title reliably behave like a button activation. |
tests/e2e/pi-web.spec.ts |
Adds regression tests for the computed CSS contract and a mobile touchscreen.tap rename interaction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+275
to
+291
| const styles = await page.locator("#statusTitle").evaluate((el) => { | ||
| const cs = getComputedStyle(el); | ||
| return { | ||
| userSelect: cs.userSelect || (cs as any).webkitUserSelect, | ||
| webkitUserSelect: (cs as any).webkitUserSelect, | ||
| webkitTouchCallout: (cs as any).webkitTouchCallout, | ||
| touchAction: cs.touchAction, | ||
| cursor: cs.cursor, | ||
| role: el.getAttribute("role"), | ||
| }; | ||
| }); | ||
| expect(styles.role).toBe("button"); | ||
| expect(styles.cursor).toBe("pointer"); | ||
| expect([styles.userSelect, styles.webkitUserSelect]).toContain("none"); | ||
| // touch-action: manipulation removes the iOS 300ms tap delay too. | ||
| expect(styles.touchAction).toBe("manipulation"); | ||
| }); |
…pointer tap The CSS-only fix (user-select:none) stopped the title <span> itself from being text-selected, but two iOS-specific issues still made the header rename unusable on iPhone: 1. iOS Safari fires a spurious `blur` on the freshly-focused rename input while the on-screen keyboard animates in / the visual viewport resizes. The old `blur -> finish(true)` handler tore the input back down to the static title before the user could type, so the tap looked like it did nothing. Now the blur-to-save handler is only armed once focus has settled (350ms); an earlier blur re-focuses the input and keeps the keyboard up. 2. The tap could still be partly consumed by iOS's selection gesture. Open the editor on `pointerup` with preventDefault (suppressing the native selection), falling back to `click` where Pointer Events are absent, with a short window to stop the synthesized click from double-triggering. Tests (tests/e2e/pi-web.spec.ts, mobile project): - extend the touch-tap test to dispatch the spurious post-focus blur within the grace window and assert the rename input survives it and still saves. Verified this fails without the blur guard (input torn down) and passes with it. Signed-off-by: Megha Goyal <goyamegh@amazon.com>
Author
VerificationReproduced on an iPhone (iOS Safari): tapping the session title in the header did not open the rename field. Root cause turned out to be three separate iOS behaviours, fixed in two commits:
Automated proof (Playwright,
|
…rkFirst nav)
Even with the rename fix shipped, iPhone clients kept running the old
bundle because the service worker layer never let the new build through.
Two root causes, both fixed here:
1. The static file server sent no Cache-Control headers, so iOS
heuristically cached /sw.js, never re-fetched the service-worker
script, never saw the new SW, and the old SW kept serving the old
precached bundle forever. Now:
- sw.js / registerSW.js / workbox-*.js -> no-cache, no-store, must-revalidate
- index.html / *.html / manifest -> no-cache
- content-hashed assets/index-*.{js,css} -> public, max-age=31536000, immutable
2. vite-plugin-pwa registers an SPA NavigationRoute bound to the precached
index.html before runtimeCaching, shadowing our NetworkFirst navigate
route. A stale SW therefore served old precached HTML (old asset hashes)
with no network fallback. Deny-list every navigation from that precache
fallback so navigations fall through to NetworkFirst (3s timeout, cached
fallback offline) — any reload now self-heals to the latest build.
Tests:
- tests/api.test.ts boots server.ts in production mode and asserts the
three Cache-Control classes (all three fail without the change).
- tests/sw-config.test.ts asserts the generated dist/sw.js denylists the
precache navigation fallback, routes navigations NetworkFirst, and keeps
skipWaiting/clientsClaim/cleanupOutdatedCaches.
Signed-off-by: Megha Goyal <goyamegh@amazon.com>
Author
Added: make iOS actually receive new buildsThe rename fix alone wasn't enough on a real iPhone — the device kept running the old bundle because the service-worker layer never let the new build through. Two root causes, both fixed (commit 1. No
|
Opt-in self-destroying service worker served at /sw.js when PI_WEB_SW_RESET=1 or a .pi-sw-reset sentinel file exists. Lets a stale, captured client (e.g. an iPhone that won't update) be forced to drop its old SW + caches server-side: the client re-fetches /sw.js on navigation, installs the kill SW, which clears caches, unregisters, and reloads once (loop-safe via a marker cache). Flip off to restore the normal PWA SW. Test asserts /sw.js is the self-destroying script under PI_WEB_SW_RESET=1. Signed-off-by: Megha Goyal <goyamegh@amazon.com>
iOS Safari only raises the keyboard when input.focus() runs inside a click/touchend gesture; focusing from pointerup (or after preventDefault) opens the field but leaves the keyboard down, so a finger tap appears to do nothing while a desktop click works. Trigger purely on click; selection is already prevented by user-select:none + touch-action:manipulation. Verified on real WebKit (iPhone 14 profile) with a genuine touch tap. Signed-off-by: Megha Goyal <goyamegh@amazon.com>
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
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.
Bug
On iPhone, tapping the session title in the top header was supposed to swap the title for an inline rename
<input>. Instead, iOS Safari treated the tap as native text-selection on the underlying<span>(showing the blue selection handles + magnifier) and the click event for the rename handler never fired, so the session could not be renamed from the header.Reproducible on iOS Safari (iPhone) — the screenshot in the bug report shows the selection handles around the title text with no rename input rendered.
Root cause
#statusTitleis a<span class="statusTitle">withrole="button"andcursor: pointer, but nouser-select: none/-webkit-touch-callout: none. iOS Safari falls back to its default text-selection gesture on selectable text, which suppresses the click handler that swaps the span for the rename input.We already use the same fix on
.sessionItemMarkerBtnfor exactly this reason.Fix
src/styles/statusBar.css— on.statusTitle:-webkit-user-select: none+user-select: none-webkit-touch-callout: nonetouch-action: manipulationThe element is already
role="button"and keyboard-activatable (Enter/Space), so disabling text selection only removes a behaviour that was incompatible with the button contract.Tests (Playwright e2e,
tests/e2e/pi-web.spec.ts).statusTitlehasuser-select: none,touch-action: manipulation,cursor: pointer, androle="button"— guards the CSS contract from regressing.touchscreen.tapon the title and assert the rename<input>becomes visible + focused, then rename through it. This is the touch-event path the mouse-based existing test does not exercise.renames the current session from the status titlestill passes on mobile / tablet / desktop.Verified the new CSS-contract test fails on
origin/mainwithout this change and passes with it.Out of scope
No behaviour change on desktop / non-touch viewports. No JS changes.