From 550e6b05a1904f40b0030bd6abca5499f76a688e Mon Sep 17 00:00:00 2001 From: Hampus Date: Mon, 14 Sep 2026 20:51:59 +0200 Subject: [PATCH] fix(app): show hover highlight and inset the focus ring (#2778) --- .../channel/components/ChannelMessage.tsx | 10 +++ .../MessageFocusRingContract.test.ts | 39 ++++++++++ .../MessageHoverStyleContract.test.ts | 15 ++++ .../features/theme/styles/Message.module.css | 77 ++++++------------- .../ui/focus_ring/FocusRingScope.test.tsx | 22 ++++++ 5 files changed, 110 insertions(+), 53 deletions(-) diff --git a/fluxer_app/src/features/channel/components/ChannelMessage.tsx b/fluxer_app/src/features/channel/components/ChannelMessage.tsx index 8532ee624..395a9dc3d 100644 --- a/fluxer_app/src/features/channel/components/ChannelMessage.tsx +++ b/fluxer_app/src/features/channel/components/ChannelMessage.tsx @@ -223,6 +223,7 @@ export const Message: React.FC = observer((props) => { const [isFocusedWithin, setIsFocusedWithin] = useState(false); const [mobileLongPressLinkUrl, setMobileLongPressLinkUrl] = useState(undefined); const messageRef = useRef(null); + const focusRingAnchorRef = useRef(null); const disableContextMenuTracking = behaviorOverrides?.disableContextMenuTracking ?? false; const trackedContextMenuOpen = useContextMenuHoverState(messageRef, !disableContextMenuTracking); const contextMenuOpen = disableContextMenuTracking @@ -706,6 +707,7 @@ export const Message: React.FC = observer((props) => { enabled={keyboardNavigationEnabled ? keyboardModeEnabled : undefined} within={keyboardNavigationEnabled} offset={keyboardNavigationEnabled ? -2 : undefined} + ringTarget={keyboardNavigationEnabled ? focusRingAnchorRef : undefined} data-flx="channel.message.focus-ring" >
= observer((props) => { style={articleStyle} data-flx="channel.message.article.alt-click" > + {keyboardNavigationEnabled && ( +
+ )} {messageComponent} {shouldMountActionBar && (previewMode ? ( diff --git a/fluxer_app/src/features/channel/components/MessageFocusRingContract.test.ts b/fluxer_app/src/features/channel/components/MessageFocusRingContract.test.ts index 86f429968..2e66a8c0a 100644 --- a/fluxer_app/src/features/channel/components/MessageFocusRingContract.test.ts +++ b/fluxer_app/src/features/channel/components/MessageFocusRingContract.test.ts @@ -14,6 +14,22 @@ const focusRingCss = readSource('../../ui/focus_ring/FocusRing.module.css'); const channelMessageSource = readSource('./ChannelMessage.tsx'); const messageFocusRing = channelMessageSource.match(/]*>/)?.[0] ?? ''; +interface CssRule { + readonly selector: string; + readonly body: string; +} + +function parseCssRules(css: string): Array { + const withoutComments = css.replace(/\/\*[\s\S]*?\*\//g, ''); + const rules: Array = []; + for (const match of withoutComments.matchAll(/([^{}]+)\{([^{}]*)\}/g)) { + rules.push({selector: match[1].replace(/\s+/g, ' ').trim(), body: match[2].trim()}); + } + return rules; +} + +const messageRules = parseCssRules(messageCss); + describe('message focus ring contract', () => { it('never draws a ring from a bare :focus selector on a message row', () => { expect(messageCss).not.toMatch(/\.message(Compact)?:focus(?!-visible)/); @@ -40,6 +56,29 @@ describe('message focus ring contract', () => { it('stacks the ring below the action bar', () => { expect(actionBarCss).toMatch(/z-index:\s*var\(--z-index-elevated-1\)/); + expect(messageFocusRing).not.toMatch(/zIndex=/); + }); + + it('measures the ring from an anchor inset by the row bleed in the experiment arm', () => { + expect(messageFocusRing).toMatch(/ringTarget=\{keyboardNavigationEnabled \? focusRingAnchorRef : undefined\}/); + expect(channelMessageSource).toMatch(/className=\{styles\.focusRingAnchor\}/); + const anchor = messageRules.find((rule) => rule.selector === '.focusRingAnchor'); + expect(anchor).toBeDefined(); + expect(anchor?.body).toMatch(/position:\s*absolute/); + expect(anchor?.body).toMatch(/inset-inline:\s*var\(--message-inline-bleed/); + expect(anchor?.body).toMatch(/pointer-events:\s*none/); + }); + + it('derives every message row bleed from the shared bleed variable', () => { + const bleedingRows = messageRules.filter( + (rule) => /^\.message(Compact|Preview)?$/.test(rule.selector) && /margin-inline\s*:/.test(rule.body), + ); + expect(bleedingRows.length).toBeGreaterThan(0); + for (const rule of bleedingRows) { + const margin = rule.body.match(/margin-inline:\s*([^;]+)/)?.[1]?.trim(); + expect(margin).toBeDefined(); + expect(margin === '0' || margin === 'calc(-1 * var(--message-inline-bleed))').toBe(true); + } }); it('falls back to a system outline under forced colors', () => { diff --git a/fluxer_app/src/features/channel/components/MessageHoverStyleContract.test.ts b/fluxer_app/src/features/channel/components/MessageHoverStyleContract.test.ts index fa5606944..4c370f68d 100644 --- a/fluxer_app/src/features/channel/components/MessageHoverStyleContract.test.ts +++ b/fluxer_app/src/features/channel/components/MessageHoverStyleContract.test.ts @@ -78,6 +78,21 @@ describe('message hover style contract', () => { } }); + it('paints the row highlight in both arms because the class is driven by React', () => { + const highlightRules = parseCssRules(messageCss).filter( + (rule) => + rule.selector.includes('.messageHovered') && + !rule.selector.includes(':hover') && + !/\.buttons\b/.test(rule.selector) && + /(background-color|opacity):/.test(rule.body), + ); + expect(highlightRules.length).toBeGreaterThan(0); + for (const rule of highlightRules) { + expect(rule.selector).not.toContain(CONTROL_GATE); + expect(rule.selector).not.toContain(TREATMENT_GATE); + } + }); + it('derives the row highlight and the action bar from the same hover state', () => { const highlightSource = channelMessageSource.match(/(\w+) && !isPreview && styles\.messageHovered/)?.[1]; const actionBarSourceState = channelMessageSource.match(/const isActionBarActive = (\w+) \|\|/)?.[1]; diff --git a/fluxer_app/src/features/theme/styles/Message.module.css b/fluxer_app/src/features/theme/styles/Message.module.css index 630964371..8c5517331 100644 --- a/fluxer_app/src/features/theme/styles/Message.module.css +++ b/fluxer_app/src/features/theme/styles/Message.module.css @@ -38,7 +38,8 @@ padding-block: var(--message-spacing-y); padding-inline-end: var(--chat-horizontal-padding, var(--chat-horizontal-padding-default)); padding-inline-start: 0; - margin-inline: calc(-1 * var(--message-mobile-margin)); + --message-inline-bleed: var(--message-mobile-margin); + margin-inline: calc(-1 * var(--message-inline-bleed)); } .messagePreviewSpacing { @@ -53,6 +54,13 @@ --message-hovered: 1; } +.focusRingAnchor { + position: absolute; + inset-block: 0; + inset-inline: var(--message-inline-bleed, 0px); + pointer-events: none; +} + .message::selection, .messageCompact::selection, .message *::selection, @@ -62,7 +70,8 @@ @media (min-width: 768px) { .message { - margin-inline: calc(-1 * var(--chat-horizontal-padding, var(--chat-horizontal-padding-default))); + --message-inline-bleed: var(--chat-horizontal-padding, var(--chat-horizontal-padding-default)); + margin-inline: calc(-1 * var(--message-inline-bleed)); padding-inline-end: calc( var(--chat-horizontal-padding, var(--chat-horizontal-padding-default)) + var(--message-action-bar-offset) @@ -70,39 +79,24 @@ } .messagePreview { + --message-inline-bleed: 0px; margin-inline: 0; padding-inline-end: var(--chat-horizontal-padding, var(--chat-horizontal-padding-default)); } } -:global( - :where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard):not( - .experiment-message-hover-tracking - ) - ) +:global(:where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard)) .message.messageHovered:not(.messageMentioned):not(.messageReplying):not(.messageHighlight):not(.messagePreview), -:global( - :where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard):not( - .experiment-message-hover-tracking - ) - ) +:global(:where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard)) .messageCompact.messageHovered:not(.messageMentioned):not(.messageReplying):not(.messageHighlight):not( .messagePreview ) { background-color: var(--background-modifier-hover); } -:global( - :where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard):not( - .experiment-message-hover-tracking - ) - ) +:global(:where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard)) .message.messageMentioned.messageHovered, -:global( - :where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard):not( - .experiment-message-hover-tracking - ) - ) +:global(:where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard)) .messageCompact.messageMentioned.messageHovered { background-color: var(--message-mention-bg-hover); } @@ -114,29 +108,13 @@ background-color: var(--message-mention-bg); } -:global( - :where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard):not( - .experiment-message-hover-tracking - ) - ) +:global(:where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard)) .message.messageReplying.messageHovered:not(.messageMentioned), -:global( - :where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard):not( - .experiment-message-hover-tracking - ) - ) +:global(:where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard)) .message.messageHighlight.messageHovered:not(.messageMentioned), -:global( - :where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard):not( - .experiment-message-hover-tracking - ) - ) +:global(:where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard)) .messageCompact.messageReplying.messageHovered:not(.messageMentioned), -:global( - :where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard):not( - .experiment-message-hover-tracking - ) - ) +:global(:where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard)) .messageCompact.messageHighlight.messageHovered:not(.messageMentioned) { background-color: var(--message-reply-bg); } @@ -218,7 +196,8 @@ padding-inline-start: 0; padding-inline-end: var(--message-compact-horizontal-padding); text-indent: 0; - margin-inline: calc(-1 * var(--chat-horizontal-padding, var(--chat-horizontal-padding-default))); + --message-inline-bleed: var(--chat-horizontal-padding, var(--chat-horizontal-padding-default)); + margin-inline: calc(-1 * var(--message-inline-bleed)); } @media (min-width: 768px) { @@ -618,11 +597,7 @@ transform: translateX(0.5rem); } -:global( - :where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard):not( - .experiment-message-hover-tracking - ) - ) +:global(:where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard)) .message.messageHovered .messageTimestampHover { opacity: 1; @@ -698,11 +673,7 @@ margin-inline-end: 0; } -:global( - :where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard):not( - .experiment-message-hover-tracking - ) - ) +:global(:where(html.window-focused, html.unfocused-fully-interactive):not(.window-focus-activation-guard)) .messageCompact.messageHovered .messageTimestampCompactHover { opacity: 1; 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 fc5181989..3d68205cf 100644 --- a/fluxer_app/src/features/ui/focus_ring/FocusRingScope.test.tsx +++ b/fluxer_app/src/features/ui/focus_ring/FocusRingScope.test.tsx @@ -287,4 +287,26 @@ describe('FocusRingScope', () => { expect(requireRingContext('primary').visible).toBe(false); expect(requireRingContext('secondary').visible).toBe(true); }); + + test('keeps a positively inset ring inside the scope container the row bleeds out of', () => { + act(() => { + root.render(); + }); + const scopeContainer = requireScopeContainer(); + stubBoundingRect(scopeContainer, () => domRect(0, 0, 500, 400)); + stubBoundingRect(requireTarget(), () => domRect(100, -16, 532, 40)); + act(() => { + requireRingContext().showForElement(requireTarget(), {offset: -2}); + }); + const bleeding = requireRing(); + expect(Number.parseFloat(bleeding.style.left)).toBeLessThan(0); + act(() => { + requireRingContext().showForElement(requireTarget(), {offset: {top: -2, bottom: -2, left: 18, right: 18}}); + }); + const inset = requireRing(); + const left = Number.parseFloat(inset.style.left); + const width = Number.parseFloat(inset.style.width); + expect(left).toBeGreaterThan(0); + expect(left + width).toBeLessThan(500); + }); });