fix(desktop): clear "New messages" indicator at bottom and per conversation (#2205) - #2211
Conversation
0732a67 to
81759db
Compare
…ersation (apache#2205) Astryx ChatLayout only cleared hasNewMessages through the button's dismiss(), so scrolling back to the bottom re-locked auto-follow but left the "New messages" label visible; and a conversation switch reused the same ChatLayout instance, leaking hasNewMessages, lastMessageRef and the unlocked scroll state into the new conversation, whose first message then re-triggered the indicator. - patches/@astryxdesign+core+0.2.0.patch: ChatLayout clears the flag on every re-lock (scrollend within the lock threshold) and gains an optional conversationKey prop that resets the scroll lock and the new-message baseline when it changes; useChatNewMessages exposes reset(). - ChatSurfaceLayout forwards conversationKey from the active session id (app-shell + quote companion). No remount: a remount would drop an in-progress composer draft, a regression composer-skill-invocation e2e caught on the earlier keyed-remount attempt. - e2e regression: new-messages-indicator.spec.ts passes with the fix and fails without it.
81759db to
0b40f6a
Compare
Resolve patches/README.md conflict by keeping both sections: - our apache#2205 "New messages" indicator section - upstream apache#2225 Astryx List accessible-name section Verified the merged @astryxdesign/core patch still applies cleanly via patch-package --error-on-fail.
|
Thanks for this — the fix is well-scoped and I verified it end to end in a browser harness against the patched dist: the badge appears on unlocked growth, clears on real P1 — the e2e selectors no longer match P3 (optional):
The fix itself is sound — happy to approve once the selectors are updated. |
Root causes of the CI e2e failures (this spec had never run on Linux
before the upstream-main merge):
1. Button names are locale-dependent — Maka's Astryx copy overrides
(astryx-copy.ts) map 'New messages'/'Scroll to bottom' to zh
('跳到最新消息'/'滚动到底部') under the default zh fixture locale.
The spec hardcoded the en names, so both scroll-button assertions
could not find the button. Match both locales — the assertion is
about the affordance, not Astryx's copy.
2. The prompt anchor rail (upstream apache#2237) renders a preview of the
sent text, so loose getByText(/Fake backend received: .../) regexes
matched two nodes (preview + transcript echo) → strict-mode
violation. Assert with { exact: true } on the echo, which the longer
preview never matches.
3. growTranscriptOverflow only required sh > ch (merely scrollable);
Astryx flags scrolled-up only past buttonThreshold (100px), so a
compact font could leave the button unrendered. Grow until
sh - ch > 150px (threshold + headroom), up to 6 messages.
Verified locally: 2/2 pass.
|
Fixed the e2e failures — root cause was not the product fix, it was the spec itself (it had never run on Linux before this merge):
Verified locally: 2/2 pass. Pushed as 243b921. |
|
Thanks — the new commit resolves everything from the prior pass: the locale-agnostic matchers cover both the zh override ( One new finding, tracked as a follow-up rather than a blocker: P2 (deferred, not blocking) — transient strict-mode double match during streaming. Between the fake backend's chunk 6 and 7 (~45ms window), the rail's reply preview normalizes+trims to exactly the query string while the transcript's first P3s (open, optional): the quote-companion Merging now — thanks for the fast turnaround! |
Fixes #2205
Summary
The desktop chat surface's "New messages" scroll-to-bottom indicator now clears when the user scrolls back to the bottom, and its state resets fully when switching conversations. Repro screenshots are attached in the issue.
Root cause
Two independent problems in
@astryxdesign/core@0.2.0:useChatNewMessagesonly clearshasNewMessagesviadismiss()(the indicator button).useChatStreamScrollre-locks auto-follow onscrollendbut never reports "at bottom" back to the indicator, so the label stays visible after the user scrolls back down.ChatLayoutis not remounted per conversation —lastMessageRef(pointing at the previous conversation's last message node) and the unlockedisLocked=falsesurvive the switch, so the first message of a new conversation re-flagshasNewMessages.Changes
apps/desktop/src/renderer/app-shell.tsx,quote-companion-panel.tsx: keyChatSurfaceLayoutper conversation so the scroll/new-message state is remounted and reset on conversation switch.patches/@astryxdesign+core+0.2.0.patch: adds the two@astryxdesign/corehunks for this fix (ChatLayout.js,useChatNewMessages.js). The file already carried the fix(ui): localize the field required/optional marker app-wide #2184FieldLabellocalization hunks, so the patch now contains both sets; verified withgit apply --checkthat the merged patch applies cleanly to a pristine@astryxdesign/core@0.2.0.patches/README.md: rationale and removal conditions for the new hunks, following the repo convention.apps/desktop/e2e/new-messages-indicator.spec.ts: Playwright regression coverage for both reported symptoms.Verification
quote-companion,send-message,scroll-geometry,sidebar-navigatione2e suites (17 tests),packages/uiunit tests (316/316), desktop typecheck and biome all clean.