-
-
Notifications
You must be signed in to change notification settings - Fork 1
fix(client): stabilize mobile send, jump recovery, and PWA diagnostics #112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
5614e43
bf7452a
9d41dc8
a01eb7e
d3303b5
8779b19
4b1fec1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| default: patch | ||
| --- | ||
|
|
||
| Fix mobile message sending, preserve jump-to-message context during timeline refreshes, and add PWA freeze diagnostics for service worker recovery. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| # Manual QA Checklist | ||
|
|
||
| Manual checks for the current Charm follow-up work on `#111`, `#103`, and `#109`. | ||
|
|
||
| ## Issue #111: Mobile send button UX | ||
|
|
||
| Environment: | ||
|
|
||
| - iPhone-sized Safari layout | ||
| - Safari standalone PWA if available | ||
| - one room with delayed events enabled | ||
| - one room or account state where delayed events are unavailable | ||
|
|
||
| Steps: | ||
|
|
||
| 1. Open a room and focus the composer so the keyboard is visible. | ||
| 2. Type a short message and tap the main send button. | ||
| 3. Confirm the message sends immediately while the keyboard is still open. | ||
| 4. Type another message and tap the separate schedule button. | ||
| 5. Close the schedule dialog without submitting. | ||
| 6. Tap the main send button again. | ||
| 7. Confirm the message sends immediately and the composer is not stuck. | ||
| 8. Re-open the schedule dialog, choose a future time, and submit it. | ||
| 9. Confirm the primary button reflects scheduled-send state and no immediate send occurs. | ||
| 10. In a context where delayed events are unavailable, confirm there is no separate schedule button and normal sending still works. | ||
|
|
||
| Expected results: | ||
|
|
||
| - main send never depends on long-press | ||
| - schedule action is explicit and separate on mobile | ||
| - closing schedule UI does not break later sends | ||
|
|
||
| ## Issue #103: Jump buttons broken | ||
|
|
||
| Environment: | ||
|
|
||
| - a room with enough history that the target event is not already in the loaded viewport | ||
| - at least one reply link, bookmark, or permalink target | ||
|
|
||
| Steps: | ||
|
|
||
| 1. Open a reply link or permalink to an older message. | ||
| 2. Confirm the target message appears before any snap back to the latest timeline. | ||
| 3. Scroll upward from the jumped position. | ||
| 4. Scroll downward from the jumped position. | ||
| 5. Repeat using a bookmark jump if available. | ||
| 6. Background the app briefly, return to foreground, and confirm the same jumped context remains stable. | ||
|
|
||
| Expected results: | ||
|
|
||
| - jump lands on the requested target | ||
| - timeline does not immediately snap to bottom | ||
| - surrounding scroll does not jitter or reset unexpectedly | ||
| - foreground return does not discard the event-targeted context too early | ||
|
|
||
| ## Issue #109: PWA freeze instrumentation | ||
|
|
||
| Environment: | ||
|
|
||
| - Safari desktop PWA preferred | ||
| - Sentry and browser console available if possible | ||
|
|
||
| Steps: | ||
|
|
||
| 1. Launch the PWA and leave it idle for an extended period. | ||
| 2. Return and attempt basic interaction: | ||
| - switch rooms | ||
| - open a DM | ||
| - click the composer | ||
| - try a normal browser reload if the UI still responds | ||
| 3. If the app freezes, note: | ||
| - whether the window still repaints | ||
| - whether clicks are ignored everywhere | ||
| - whether reload is possible | ||
| 4. Capture any visible console output and the latest Sentry breadcrumbs/metrics around: | ||
| - app visibility changes | ||
| - pageshow restore | ||
| - service worker controller changes | ||
| - service worker claim requests | ||
| - background client startup or failure | ||
| - forced reload requests | ||
|
|
||
| Expected results: | ||
|
|
||
| - enough telemetry exists to classify the freeze as controller churn, restore failure, background-client deadlock, or sync/network stall |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import type { ReactNode } from 'react'; | ||
| import { render, screen } from '@testing-library/react'; | ||
| import { describe, expect, it } from 'vitest'; | ||
| import type { Editor } from 'slate'; | ||
| import { vi } from 'vitest'; | ||
|
|
||
| import { AutocompleteMenu } from './AutocompleteMenu'; | ||
|
|
||
| vi.mock('focus-trap-react', () => ({ | ||
| default: ({ children }: { children: ReactNode }) => children, | ||
| })); | ||
|
|
||
| vi.mock('slate-react', () => ({ | ||
| ReactEditor: { | ||
| focus: vi.fn(), | ||
| }, | ||
| })); | ||
|
|
||
| describe('AutocompleteMenu', () => { | ||
| const editor = {} as Editor; | ||
|
|
||
| it('marks the first item selected by default', () => { | ||
| render( | ||
| <AutocompleteMenu headerContent="Test" requestClose={vi.fn()} editor={editor}> | ||
| <button type="button">First</button> | ||
| <button type="button">Second</button> | ||
| </AutocompleteMenu> | ||
| ); | ||
|
|
||
| expect(screen.getByRole('button', { name: 'First' })).toHaveAttribute('data-selected', 'true'); | ||
| expect(screen.getByRole('button', { name: 'Second' })).toHaveAttribute( | ||
| 'data-selected', | ||
| 'false' | ||
| ); | ||
| }); | ||
|
|
||
| it('updates the selected item when autocomplete-navigate is dispatched', () => { | ||
| const { container } = render( | ||
| <AutocompleteMenu headerContent="Test" requestClose={vi.fn()} editor={editor}> | ||
| <button type="button">First</button> | ||
| <button type="button">Second</button> | ||
| <button type="button">Third</button> | ||
| </AutocompleteMenu> | ||
| ); | ||
|
|
||
| const menu = container.querySelector('[data-autocomplete-menu]'); | ||
| expect(menu).not.toBeNull(); | ||
|
|
||
| menu!.dispatchEvent(new CustomEvent('autocomplete-navigate', { detail: { direction: 1 } })); | ||
|
|
||
| expect(screen.getByRole('button', { name: 'Second' })).toHaveAttribute('data-selected', 'true'); | ||
| expect(screen.getByRole('button', { name: 'First' })).toHaveAttribute('data-selected', 'false'); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import type { ReactNode } from 'react'; | ||
| import { useEffect, useRef, useState } from 'react'; | ||
| import { useCallback, useEffect, useRef, useState } from 'react'; | ||
| import FocusTrap from 'focus-trap-react'; | ||
| import { isKeyHotkey } from 'is-hotkey'; | ||
| import { Header, Menu, Scroll, config } from 'folds'; | ||
|
|
@@ -25,6 +25,7 @@ | |
| }: AutocompleteMenuProps) { | ||
| const alive = useAlive(); | ||
| const itemsRef = useRef<HTMLDivElement>(null); | ||
| const selectedIndexRef = useRef(0); | ||
|
|
||
| const handleDeactivate = () => { | ||
| if (alive()) { | ||
|
|
@@ -34,6 +35,75 @@ | |
| }; | ||
| const [isActive, setIsActive] = useState(true); | ||
| useEffect(() => ReactEditor.focus(editor), [editor, isActive]); | ||
|
|
||
| const applySelectedIndex = useCallback((nextIndex: number, focus = false) => { | ||
| const buttons = Array.from( | ||
| itemsRef.current?.querySelectorAll<HTMLButtonElement>('button') ?? [] | ||
| ); | ||
| if (buttons.length === 0) { | ||
| selectedIndexRef.current = 0; | ||
| return; | ||
| } | ||
|
|
||
| const clampedIndex = Math.max(0, Math.min(nextIndex, buttons.length - 1)); | ||
| selectedIndexRef.current = clampedIndex; | ||
|
|
||
| buttons.forEach((button, index) => { | ||
| button.dataset.selected = String(index === clampedIndex); | ||
| }); | ||
|
|
||
| if (focus) { | ||
| const selectedButton = buttons[clampedIndex]; | ||
| selectedButton?.focus({ preventScroll: true }); | ||
| selectedButton?.scrollIntoView?.({ block: 'nearest' }); | ||
| } | ||
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| applySelectedIndex(0); | ||
| }, [children, applySelectedIndex]); | ||
|
|
||
| useEffect(() => { | ||
| const items = itemsRef.current; | ||
| const menuRoot = items?.closest('[data-autocomplete-menu]'); | ||
| if (!items || !menuRoot) return undefined; | ||
|
|
||
| const handleNavigate = (event: Event) => { | ||
| const direction = Number( | ||
| (event as CustomEvent<{ direction?: number }>).detail?.direction ?? 0 | ||
| ); | ||
| if (!Number.isFinite(direction) || direction === 0) return; | ||
| applySelectedIndex(selectedIndexRef.current + direction, true); | ||
| }; | ||
|
|
||
| const handleFocusIn = (event: FocusEvent) => { | ||
| const target = event.target; | ||
| if (!(target instanceof HTMLButtonElement)) return; | ||
| const buttons = Array.from(items.querySelectorAll<HTMLButtonElement>('button')); | ||
| const index = buttons.indexOf(target); | ||
| if (index >= 0) applySelectedIndex(index); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When focus is still in the editor and the user presses ArrowUp as the first autocomplete navigation key, the active FocusTrap handles ArrowUp in the capture phase, moves focus to the last button, and this new Useful? React with 👍 / 👎. |
||
| }; | ||
|
|
||
| const handlePointerMove = (event: PointerEvent) => { | ||
| const target = event.target; | ||
| if (!(target instanceof Element)) return; | ||
| const button = target.closest<HTMLButtonElement>('button'); | ||
| if (!button) return; | ||
| const buttons = Array.from(items.querySelectorAll<HTMLButtonElement>('button')); | ||
| const index = buttons.indexOf(button); | ||
| if (index >= 0) applySelectedIndex(index); | ||
| }; | ||
|
|
||
| menuRoot.addEventListener('autocomplete-navigate', handleNavigate as EventListener); | ||
| items.addEventListener('focusin', handleFocusIn); | ||
| items.addEventListener('pointermove', handlePointerMove); | ||
| return () => { | ||
| menuRoot.removeEventListener('autocomplete-navigate', handleNavigate as EventListener); | ||
| items.removeEventListener('focusin', handleFocusIn); | ||
| items.removeEventListener('pointermove', handlePointerMove); | ||
| }; | ||
| }, [applySelectedIndex]); | ||
|
|
||
| function handleInput(evt: KeyboardEvent) { | ||
| if (!evt) return; | ||
| if ( | ||
|
|
@@ -71,7 +141,11 @@ | |
| {headerContent} | ||
| </Header> | ||
| <Scroll style={{ flexGrow: 1 }} onKeyDown={preventScrollWithArrowKey}> | ||
| <div ref={itemsRef} style={{ padding: config.space.S200 }}> | ||
| <div | ||
| ref={itemsRef} | ||
| className={css.AutocompleteMenuItems} | ||
| style={{ padding: config.space.S200 }} | ||
| > | ||
| {children} | ||
| </div> | ||
| </Scroll> | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.