From 1861432a53605fa6e384649fa1f307744a5a69bc Mon Sep 17 00:00:00 2001 From: Hampus Date: Mon, 14 Sep 2026 21:44:12 +0200 Subject: [PATCH] fix(app): scope message rings and reach the composer by key (#2781) --- .../handlers/defaultHandlers.ts | 3 +- .../channel/components/ChannelMessage.tsx | 1 - .../channel/components/ChannelMessages.tsx | 30 +++++--- .../MessageFocusRingContract.test.ts | 14 +++- .../useMessageListKeyboardNavigation.test.tsx | 62 +++++++++++++++- .../hooks/useMessageListKeyboardNavigation.ts | 5 ++ .../utils/ChannelTextareaFocusUtils.test.ts | 48 +++++++++++++ .../utils/ChannelTextareaFocusUtils.ts | 8 +++ .../ui/focus_ring/FocusRingScope.test.tsx | 70 +++++++++++++++++++ 9 files changed, 228 insertions(+), 13 deletions(-) create mode 100644 fluxer_app/src/features/messaging/utils/ChannelTextareaFocusUtils.test.ts diff --git a/fluxer_app/src/features/app/keybindings/keybind_manager/handlers/defaultHandlers.ts b/fluxer_app/src/features/app/keybindings/keybind_manager/handlers/defaultHandlers.ts index 1a94e1281..ca367ef56 100644 --- a/fluxer_app/src/features/app/keybindings/keybind_manager/handlers/defaultHandlers.ts +++ b/fluxer_app/src/features/app/keybindings/keybind_manager/handlers/defaultHandlers.ts @@ -23,6 +23,7 @@ import {KeyboardShortcutsCheatsheetModal} from '@app/features/input/components/m import Keybind, {type KeybindCommand} from '@app/features/input/state/InputKeybind'; import MessageEdit from '@app/features/messaging/state/MessageEdit'; import SavedMessages from '@app/features/messaging/state/SavedMessages'; +import {focusChannelTextareaFromKeybind} from '@app/features/messaging/utils/ChannelTextareaFocusUtils'; import {openExternalUrlWithWarning} from '@app/features/messaging/utils/ExternalLinkUtils'; import {buildChannelLink} from '@app/features/messaging/utils/MessageLinkUtils'; import {goToMessage} from '@app/features/messaging/utils/MessageNavigator'; @@ -537,7 +538,7 @@ export function registerDefaultKeybindHandlers(host: HandlerHost, i18n: I18n): v if (type !== 'press') return; const channelId = host.currentChannelId; if (!channelId) return; - ComponentBus.dispatch('FOCUS_TEXTAREA', {channelId}); + focusChannelTextareaFromKeybind(channelId); }); host.register('chat_upload', ({type}) => { if (type !== 'press') return; diff --git a/fluxer_app/src/features/channel/components/ChannelMessage.tsx b/fluxer_app/src/features/channel/components/ChannelMessage.tsx index 395a9dc3d..28afe8e52 100644 --- a/fluxer_app/src/features/channel/components/ChannelMessage.tsx +++ b/fluxer_app/src/features/channel/components/ChannelMessage.tsx @@ -705,7 +705,6 @@ export const Message: React.FC = observer((props) => { <> { + ComponentBus.dispatch('FOCUS_TEXTAREA', {channelId: channel.id, enterKeyboardMode: true}); + }, allowWhenInactive: true, }); useEffect(() => { @@ -666,6 +670,17 @@ export const Messages = observer(function Messages({ /> ); + const keyboardNavigationEnabled = MessageKeyboardFocusRollout.enabled; + const messageListContent = ( + + + {scrollerInner} + + + ); return (
- - - {scrollerInner} - - + {keyboardNavigationEnabled ? ( + + {messageListContent} + + ) : ( + messageListContent + )}
diff --git a/fluxer_app/src/features/channel/components/MessageFocusRingContract.test.ts b/fluxer_app/src/features/channel/components/MessageFocusRingContract.test.ts index 2e66a8c0a..03d60b38f 100644 --- a/fluxer_app/src/features/channel/components/MessageFocusRingContract.test.ts +++ b/fluxer_app/src/features/channel/components/MessageFocusRingContract.test.ts @@ -12,6 +12,7 @@ const messageCss = readSource('../../theme/styles/Message.module.css'); const actionBarCss = readSource('./MessageActionBar.module.css'); const focusRingCss = readSource('../../ui/focus_ring/FocusRing.module.css'); const channelMessageSource = readSource('./ChannelMessage.tsx'); +const channelMessagesSource = readSource('./ChannelMessages.tsx'); const messageFocusRing = channelMessageSource.match(/]*>/)?.[0] ?? ''; interface CssRule { @@ -46,7 +47,18 @@ describe('message focus ring contract', () => { it('only enables the ring in keyboard navigation mode in the experiment arm', () => { expect(messageFocusRing).toMatch(/enabled=\{keyboardNavigationEnabled \? keyboardModeEnabled : undefined\}/); - expect(messageFocusRing).toMatch(/within=\{keyboardNavigationEnabled\}/); + }); + + it('leaves focused descendants their own rings instead of drawing the row ring over them', () => { + expect(messageFocusRing).not.toMatch(/\bwithin\b/); + }); + + it('scopes message list rings inside the scroll content in the experiment arm', () => { + const scopedList = channelMessagesSource.match( + /\{keyboardNavigationEnabled \? \(\s*]*>\s*\{messageListContent\}\s*<\/FocusRingScope>\s*\) : \(\s*messageListContent\s*\)\}/, + ); + expect(scopedList).not.toBeNull(); + expect(channelMessagesSource).toMatch(/const keyboardNavigationEnabled = MessageKeyboardFocusRollout\.enabled;/); }); it('insets the ring inside the row in the experiment arm and keeps the default geometry in control', () => { diff --git a/fluxer_app/src/features/messaging/hooks/useMessageListKeyboardNavigation.test.tsx b/fluxer_app/src/features/messaging/hooks/useMessageListKeyboardNavigation.test.tsx index 2f481e27e..928d9a0e0 100644 --- a/fluxer_app/src/features/messaging/hooks/useMessageListKeyboardNavigation.test.tsx +++ b/fluxer_app/src/features/messaging/hooks/useMessageListKeyboardNavigation.test.tsx @@ -47,10 +47,22 @@ function focusedRowId(): string | null { return active instanceof HTMLElement ? (active.dataset.messageId ?? null) : null; } -function render(onFocusMessage?: (messageId: string) => void): void { +interface EdgeOptions { + onNavigatePastNewest?: () => void; + onLoadMoreAfter?: () => void; + hasMoreAfter?: boolean; +} + +function render(onFocusMessage?: (messageId: string) => void, edge: EdgeOptions = {}): void { const containerRef: RefObject = {current: viewport}; function Harness(): null { - useMessageListKeyboardNavigation({containerRef, channelId: CHANNEL_ID, onFocusMessage, allowWhenInactive: true}); + useMessageListKeyboardNavigation({ + containerRef, + channelId: CHANNEL_ID, + onFocusMessage, + allowWhenInactive: true, + ...edge, + }); return null; } act(() => { @@ -219,6 +231,52 @@ describe('useMessageListKeyboardNavigation', () => { }); }); +describe('useMessageListKeyboardNavigation past the newest message', () => { + const focusRow = (messageId: string) => { + findMessageElement(document, viewport, CHANNEL_ID, messageId)?.focus({preventScroll: true}); + }; + + it('hands focus to the composer when stepping down past the newest message', () => { + mountRows([ + {messageId: '1', idPrefix: CHANNEL_MESSAGE_ID_PREFIX}, + {messageId: '2', idPrefix: CHANNEL_MESSAGE_ID_PREFIX}, + ]); + const onNavigatePastNewest = vi.fn(); + render(focusRow, {onNavigatePastNewest}); + + pressArrow('ArrowDown'); + pressArrow('ArrowDown'); + expect(focusedRowId()).toBe('2'); + expect(onNavigatePastNewest).not.toHaveBeenCalled(); + + pressArrow('ArrowDown'); + expect(onNavigatePastNewest).toHaveBeenCalledTimes(1); + }); + + it('loads newer messages instead of leaving the list while more exist after it', () => { + mountRows([{messageId: '1', idPrefix: CHANNEL_MESSAGE_ID_PREFIX}]); + const onNavigatePastNewest = vi.fn(); + const onLoadMoreAfter = vi.fn(); + render(focusRow, {onNavigatePastNewest, onLoadMoreAfter, hasMoreAfter: true}); + + pressArrow('ArrowDown'); + pressArrow('ArrowDown'); + expect(onLoadMoreAfter).toHaveBeenCalledTimes(1); + expect(onNavigatePastNewest).not.toHaveBeenCalled(); + }); + + it('stays on the newest message in the control arm', () => { + rolloutMock.enabled = false; + mountRows([{messageId: '1', idPrefix: CHANNEL_MESSAGE_ID_PREFIX}]); + const onNavigatePastNewest = vi.fn(); + render(focusRow, {onNavigatePastNewest}); + + pressArrow('ArrowDown'); + pressArrow('ArrowDown'); + expect(onNavigatePastNewest).not.toHaveBeenCalled(); + }); +}); + describe('useMessageListKeyboardNavigation control arm', () => { beforeEach(() => { rolloutMock.enabled = false; diff --git a/fluxer_app/src/features/messaging/hooks/useMessageListKeyboardNavigation.ts b/fluxer_app/src/features/messaging/hooks/useMessageListKeyboardNavigation.ts index c3f53ae93..8b4ab8ebe 100644 --- a/fluxer_app/src/features/messaging/hooks/useMessageListKeyboardNavigation.ts +++ b/fluxer_app/src/features/messaging/hooks/useMessageListKeyboardNavigation.ts @@ -18,6 +18,7 @@ interface MessageListKeyboardNavigationOptions { hasMoreAfter?: boolean; isLoadingMore?: boolean; onEscape?: () => void; + onNavigatePastNewest?: () => void; allowWhenInactive?: boolean; } @@ -67,6 +68,7 @@ export function useMessageListKeyboardNavigation(options: MessageListKeyboardNav hasMoreAfter = false, isLoadingMore = false, onEscape, + onNavigatePastNewest, allowWhenInactive = false, } = options; const keyboardModeEnabled = KeyboardMode.keyboardModeEnabled; @@ -168,6 +170,8 @@ export function useMessageListKeyboardNavigation(options: MessageListKeyboardNav if (nextIdx >= nodes.length) { if (hasMoreAfter && onLoadMoreAfter && !isLoadingMore) { onLoadMoreAfter(); + } else if (keyboardNavigationEnabled && !hasMoreAfter && onNavigatePastNewest) { + onNavigatePastNewest(); } return; } @@ -241,6 +245,7 @@ export function useMessageListKeyboardNavigation(options: MessageListKeyboardNav hasMoreAfter, isLoadingMore, onEscape, + onNavigatePastNewest, allowWhenInactive, ]); } diff --git a/fluxer_app/src/features/messaging/utils/ChannelTextareaFocusUtils.test.ts b/fluxer_app/src/features/messaging/utils/ChannelTextareaFocusUtils.test.ts new file mode 100644 index 000000000..cf56b3f18 --- /dev/null +++ b/fluxer_app/src/features/messaging/utils/ChannelTextareaFocusUtils.test.ts @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +import {readFileSync} from 'node:fs'; +import {fileURLToPath} from 'node:url'; +import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; + +const rolloutMock = {enabled: true}; +const dispatch = vi.fn(); + +vi.mock('@app/features/messaging/state/MessageKeyboardFocusRollout', () => ({default: rolloutMock})); +vi.mock('@app/features/platform/utils/ComponentBus', () => ({ComponentBus: {dispatch}})); + +const {focusChannelTextareaFromKeybind} = await import('@app/features/messaging/utils/ChannelTextareaFocusUtils'); + +const CHANNEL_ID = '900000000000000001'; + +beforeEach(() => { + rolloutMock.enabled = true; + dispatch.mockReset(); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('focusChannelTextareaFromKeybind', () => { + it('keeps keyboard mode on when the keybind focuses the composer in the experiment arm', () => { + focusChannelTextareaFromKeybind(CHANNEL_ID); + expect(dispatch).toHaveBeenCalledWith('FOCUS_TEXTAREA', {channelId: CHANNEL_ID, enterKeyboardMode: true}); + }); + + it('dispatches the unchanged payload in the control arm', () => { + rolloutMock.enabled = false; + focusChannelTextareaFromKeybind(CHANNEL_ID); + expect(dispatch).toHaveBeenCalledWith('FOCUS_TEXTAREA', {channelId: CHANNEL_ID}); + }); + + it('is what the Tab bound chat_focus_textarea action runs', () => { + const handlers = readFileSync( + fileURLToPath(new URL('../../app/keybindings/keybind_manager/handlers/defaultHandlers.ts', import.meta.url)), + 'utf8', + ); + const handler = handlers.match(/host\.register\('chat_focus_textarea'[\s\S]*?\n\t\}\);/)?.[0]; + expect(handler).toBeDefined(); + expect(handler).toMatch(/focusChannelTextareaFromKeybind\(channelId\)/); + expect(handler).not.toMatch(/ComponentBus\.dispatch/); + }); +}); diff --git a/fluxer_app/src/features/messaging/utils/ChannelTextareaFocusUtils.ts b/fluxer_app/src/features/messaging/utils/ChannelTextareaFocusUtils.ts index cf0148315..3453b13ea 100644 --- a/fluxer_app/src/features/messaging/utils/ChannelTextareaFocusUtils.ts +++ b/fluxer_app/src/features/messaging/utils/ChannelTextareaFocusUtils.ts @@ -1,5 +1,6 @@ // SPDX-License-Identifier: AGPL-3.0-or-later +import MessageKeyboardFocusRollout from '@app/features/messaging/state/MessageKeyboardFocusRollout'; import {ComponentBus} from '@app/features/platform/utils/ComponentBus'; export function focusChannelTextareaAfterNavigation(channelId: string): void { @@ -9,3 +10,10 @@ export function focusChannelTextareaAfterNavigation(channelId: string): void { window.requestAnimationFrame(requestFocus); window.setTimeout(requestFocus, 300); } + +export function focusChannelTextareaFromKeybind(channelId: string): void { + ComponentBus.dispatch( + 'FOCUS_TEXTAREA', + MessageKeyboardFocusRollout.enabled ? {channelId, enterKeyboardMode: true} : {channelId}, + ); +} diff --git a/fluxer_app/src/features/ui/focus_ring/FocusRingScope.test.tsx b/fluxer_app/src/features/ui/focus_ring/FocusRingScope.test.tsx index 3d68205cf..5cebf4963 100644 --- a/fluxer_app/src/features/ui/focus_ring/FocusRingScope.test.tsx +++ b/fluxer_app/src/features/ui/focus_ring/FocusRingScope.test.tsx @@ -1,6 +1,7 @@ // @vitest-environment happy-dom // SPDX-License-Identifier: AGPL-3.0-or-later +import FocusRing from '@app/features/ui/focus_ring/FocusRing'; import FocusRingContext, {type FocusRingContextManager} from '@app/features/ui/focus_ring/FocusRingContext'; import FocusRingManager from '@app/features/ui/focus_ring/FocusRingManager'; import FocusRingScope from '@app/features/ui/focus_ring/FocusRingScope'; @@ -310,3 +311,72 @@ describe('FocusRingScope', () => { expect(left + width).toBeLessThan(500); }); }); + +function NestedRings({outerWithin}: {outerWithin: boolean}) { + const containerRef = useRef(null); + return ( +
+ + + +
+
+ + + +
+
+ + +
+ ); +} + +function requireElement(selector: string): HTMLElement { + const element = container.querySelector(selector); + if (element == null) throw new Error(`Missing ${selector}`); + return element; +} + +describe('FocusRing nested inside another ring', () => { + test('a focus-within ring replaces the ring of a descendant that draws its own', () => { + act(() => { + root.render(); + }); + act(() => { + requireElement('button').focus(); + }); + expect(requireRingContext().targetElement).toBe(requireElement('[data-row]')); + }); + + test('a ring without focus-within leaves the descendant its own ring', () => { + act(() => { + root.render(); + }); + act(() => { + requireElement('button').focus(); + }); + expect(requireRingContext().targetElement).toBe(requireElement('button')); + }); + + test('stacks a descendant ring above an elevated bar and the row ring beneath it', () => { + act(() => { + root.render(); + }); + act(() => { + requireElement('button').focus(); + }); + expect(requireRing().style.zIndex).toBe('11'); + act(() => { + requireElement('[data-row]').focus(); + }); + expect(requireRingContext().targetElement).toBe(requireElement('[data-row]')); + expect(requireRing().style.zIndex).toBe(''); + }); +});