Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions desktop/src/features/forum/ui/ForumComposer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { ChevronDown } from "lucide-react";
import { buildOutgoingMessage } from "@/features/messages/lib/imetaMediaMarkdown";
import { useChannelLinks } from "@/features/messages/lib/useChannelLinks";
import type { ChannelSuggestion } from "@/features/messages/lib/useChannelLinks";
import { useComposerFocusOwnership } from "@/features/messages/lib/useComposerFocusOwnership";
import { useMediaUpload } from "@/features/messages/lib/useMediaUpload";
import { isMentionCodeContext } from "@/features/messages/lib/mentionCodeContext";
import { useMentions } from "@/features/messages/lib/useMentions";
Expand Down Expand Up @@ -101,6 +102,8 @@ export function ForumComposer({
mentions.isMentionOpen || channelLinks.isChannelOpen;

const submitMessageRef = React.useRef<() => void>(() => {});
const formRef = React.useRef<HTMLFormElement>(null);
const composerOwnsFocus = useComposerFocusOwnership(formRef);

// Set after `useLinkEditor` exists; the editor's link-click handler
// delegates through this ref to break the hook ordering cycle.
Expand Down Expand Up @@ -500,6 +503,7 @@ export function ForumComposer({
}}
onFocusCapture={expandCompactComposer}
onSubmit={handleSubmit}
ref={formRef}
>
{media.isDragOver && <DropZoneOverlay />}
{isCompactLayout ? (
Expand All @@ -526,6 +530,7 @@ export function ForumComposer({
? channelLinks.channelSuggestions
: []
}
composerOwnsFocus={composerOwnsFocus}
mentionSelectedIndex={mentions.mentionSelectedIndex}
mentionSuggestions={
mentions.isMentionOpen ? mentions.suggestions : []
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
type ForumComposerAutocompletesProps = {
channelSelectedIndex: number;
channelSuggestions: ChannelSuggestion[];
composerOwnsFocus: boolean;
mentionSelectedIndex: number;
mentionSuggestions: MentionSuggestion[];
onChannelSelect: (suggestion: ChannelSuggestion) => void;
Expand All @@ -20,6 +21,7 @@ type ForumComposerAutocompletesProps = {
export function ForumComposerAutocompletes({
channelSelectedIndex,
channelSuggestions,
composerOwnsFocus,
mentionSelectedIndex,
mentionSuggestions,
onChannelSelect,
Expand All @@ -31,12 +33,14 @@ export function ForumComposerAutocompletes({
return (
<>
<ChannelAutocomplete
composerOwnsFocus={composerOwnsFocus}
onSelect={onChannelSelect}
position={position}
selectedIndex={channelSelectedIndex}
suggestions={channelSuggestions}
/>
<MentionAutocomplete
composerOwnsFocus={composerOwnsFocus}
onDismiss={onMentionDismiss}
onFetchMore={onMentionFetchMore}
onSelect={onMentionSelect}
Expand Down
5 changes: 4 additions & 1 deletion desktop/src/features/messages/lib/useChannelLinks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,11 @@ export function useChannelLinks() {
return { handled: true };
}

// Forward Tab selects; Shift+Tab deliberately does not. The reverse
// move stays the browser's, so this overlay can't swallow a keyboard
// user's way back out (see useMentions for the same split).
if (
event.key === "Tab" ||
(event.key === "Tab" && !event.shiftKey) ||
(event.key === "Enter" &&
!event.ctrlKey &&
!event.metaKey &&
Expand Down
121 changes: 121 additions & 0 deletions desktop/src/features/messages/lib/useComposerFocusOwnership.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
import assert from "node:assert/strict";
import { after, afterEach, before, test } from "node:test";

import { JSDOM } from "jsdom";

const dom = new JSDOM("<!doctype html><html><body></body></html>", {
url: "http://localhost",
});

before(() => {
Object.assign(globalThis, {
document: dom.window.document,
Element: dom.window.Element,
Event: dom.window.Event,
FocusEvent: dom.window.FocusEvent,
getComputedStyle: dom.window.getComputedStyle.bind(dom.window),
HTMLElement: dom.window.HTMLElement,
IS_REACT_ACT_ENVIRONMENT: true,
Node: dom.window.Node,
window: dom.window,
});
});

afterEach(async () => {
const { cleanup } = await import("@testing-library/react");
cleanup();
});

after(() => dom.window.close());

async function renderHarness() {
const React = await import("react");
const { render } = await import("@testing-library/react");
const { useComposerFocusOwnership } = await import(
"./useComposerFocusOwnership.ts"
);

function Harness() {
const formRef = React.useRef(null);
const ownsFocus = useComposerFocusOwnership(formRef);
return React.createElement(
React.Fragment,
null,
React.createElement(
"form",
{ "data-testid": "composer", ref: formRef },
React.createElement("input", { "aria-label": "Editor" }),
React.createElement("button", { type: "button" }, "Overlay control"),
React.createElement("output", {
"data-testid": "owned",
"data-owned": String(ownsFocus),
}),
),
React.createElement("input", { "aria-label": "Elsewhere" }),
);
}

const view = render(React.createElement(Harness));
return {
view,
ownership: () => view.getByTestId("owned").getAttribute("data-owned"),
};
}

test("tracks focus entering, moving within, and leaving the composer", async () => {
const { act } = await import("react");
const { view, ownership } = await renderHarness();

assert.equal(ownership(), "false");

const editor = view.getByRole("textbox", { name: "Editor" });
await act(async () => editor.focus());
assert.equal(ownership(), "true");

// Focus handed from the editor to an overlay control stays owned — this is
// the transition an editor-focus gate got wrong, unmounting the overlay
// before the control it was handing focus to could receive it.
const control = view.getByRole("button", { name: "Overlay control" });
await act(async () => control.focus());
assert.equal(ownership(), "true");

const elsewhere = view.getByRole("textbox", { name: "Elsewhere" });
await act(async () => elsewhere.focus());
assert.equal(ownership(), "false");
});

test("an internal focus move never reports an unowned intermediate state", async () => {
const React = await import("react");
const { act } = React;
const { render } = await import("@testing-library/react");
const { useComposerFocusOwnership } = await import(
"./useComposerFocusOwnership.ts"
);

const observed = [];
function Harness() {
const formRef = React.useRef(null);
const ownsFocus = useComposerFocusOwnership(formRef);
observed.push(ownsFocus);
return React.createElement(
"form",
{ ref: formRef },
React.createElement("input", { "aria-label": "Editor" }),
React.createElement("button", { type: "button" }, "Overlay control"),
);
}

const view = render(React.createElement(Harness));
const editor = view.getByRole("textbox", { name: "Editor" });
const control = view.getByRole("button", { name: "Overlay control" });
await act(async () => editor.focus());
observed.length = 0;

// relatedTarget mirrors a browser handing focus editor → overlay control.
// The focusout handler must read it instead of assuming focus left.
const { fireEvent } = await import("@testing-library/react");
fireEvent.focusOut(editor, { relatedTarget: control });
fireEvent.focusIn(control);

assert.equal(observed.includes(false), false);
});
44 changes: 44 additions & 0 deletions desktop/src/features/messages/lib/useComposerFocusOwnership.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import * as React from "react";

/**
* Tracks whether a composer owns document focus: true while the focused
* element lives anywhere inside `containerRef` (the composer form) — the
* editor or the focusable controls of its suggestion overlays.
*
* This is the value the autocomplete overlays gate their rendering on. It is
* deliberately not the editor's own focus state: an overlay gated on editor
* focus alone unmounts the moment keyboard focus moves from the editor into
* the overlay's controls, which makes those controls unreachable. Ownership
* is tracked with `focusout` + `relatedTarget` containment rather than
* blur/focus pairs so an internal focus move never passes through a false
* state — a false flicker would unmount the overlay before the control it
* is handing focus to receives it. Each composer form has its own instance,
* so focus in one composer never keeps a sibling composer's overlays alive.
*/
export function useComposerFocusOwnership(
containerRef: React.RefObject<HTMLElement | null>,
): boolean {
const [ownsFocus, setOwnsFocus] = React.useState(false);

React.useEffect(() => {
const container = containerRef.current;
if (!container) return;

setOwnsFocus(container.contains(document.activeElement));
const handleFocusIn = () => setOwnsFocus(true);
const handleFocusOut = (event: FocusEvent) => {
setOwnsFocus(
event.relatedTarget instanceof Node &&
container.contains(event.relatedTarget),
);
};
container.addEventListener("focusin", handleFocusIn);
container.addEventListener("focusout", handleFocusOut);
return () => {
container.removeEventListener("focusin", handleFocusIn);
container.removeEventListener("focusout", handleFocusOut);
};
}, [containerRef]);

return ownsFocus;
}
5 changes: 4 additions & 1 deletion desktop/src/features/messages/lib/useEmojiAutocomplete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,11 @@ export function useEmojiAutocomplete(customEmoji: CustomEmoji[] = []) {
return { handled: true };
}

// Forward Tab selects; Shift+Tab deliberately does not. The reverse
// move stays the browser's, so this overlay can't swallow a keyboard
// user's way back out (see useMentions for the same split).
if (
event.key === "Tab" ||
(event.key === "Tab" && !event.shiftKey) ||
(event.key === "Enter" &&
!event.ctrlKey &&
!event.metaKey &&
Expand Down
6 changes: 5 additions & 1 deletion desktop/src/features/messages/lib/useMentions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -749,9 +749,13 @@ export function useMentions(
);
return { handled: true };
}
// Shift+Tab is deliberately not a select: it is the keyboard route out
// of the editor — into this overlay's Options controls where the
// composer offers them, otherwise the browser's own backward focus
// move — so those controls stay reachable.
if (
exactMentionSpace ||
event.key === "Tab" ||
(event.key === "Tab" && !event.shiftKey) ||
(event.key === "Enter" &&
!event.ctrlKey &&
!event.metaKey &&
Expand Down
14 changes: 8 additions & 6 deletions desktop/src/features/messages/lib/useRichTextEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,16 +167,12 @@ export function useRichTextEditor({
const addressedAgentMentionNamesRef = React.useRef<readonly string[]>([]);
const onUpdateRef = React.useRef(onUpdate);
onUpdateRef.current = onUpdate;

const onSubmitRef = React.useRef(onSubmit);
onSubmitRef.current = onSubmit;

const onEditLastOwnMessageRef = React.useRef(onEditLastOwnMessage);
onEditLastOwnMessageRef.current = onEditLastOwnMessage;

const onEditLinkRef = React.useRef(onEditLink);
onEditLinkRef.current = onEditLink;

const onLinkSelectionChangeRef = React.useRef(onLinkSelectionChange);
onLinkSelectionChangeRef.current = onLinkSelectionChange;

Expand Down Expand Up @@ -618,13 +614,19 @@ export function useRichTextEditor({
const hadFocusBeforeDisableRef = React.useRef(false);
React.useEffect(() => {
if (!editor || editor.isEditable === editable) return;
// `emitUpdate: false` on both toggles — the doc hasn't changed, so the
// default synthetic `update` event would replay `onUpdate` with stale
// text/cursor and resurrect consumer state derived from it (e.g. reopen
// a mention menu the user dismissed with Escape, or re-fire a typing
// notification for an untouched draft). Real content changes (typing,
// clearContent) dispatch real transactions that emit their own updates.
if (!editable) {
// About to disable: remember whether we currently hold focus so we know
// whether to restore it when re-enabled.
hadFocusBeforeDisableRef.current = editor.isFocused;
editor.setEditable(false);
editor.setEditable(false, false);
} else {
editor.setEditable(true);
editor.setEditable(true, false);
// Re-enabled: if we owned focus before the disable blurred us, take it
// back (preserving the current selection — `focus()` with no arg keeps
// the existing selection rather than jumping to the end).
Expand Down
10 changes: 9 additions & 1 deletion desktop/src/features/messages/ui/ChannelAutocomplete.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,19 @@ import {
type ChannelAutocompleteProps = {
suggestions: ChannelSuggestion[];
selectedIndex: number;
/**
* Whether the owning composer owns document focus. Composers that don't
* must not render suggestions — see MentionAutocomplete for the rationale.
*/
composerOwnsFocus: boolean;
onSelect: (suggestion: ChannelSuggestion) => void;
position?: "above" | "below";
};

export const ChannelAutocomplete = React.memo(function ChannelAutocomplete({
suggestions,
selectedIndex,
composerOwnsFocus,
onSelect,
position = "above",
}: ChannelAutocompleteProps) {
Expand All @@ -31,7 +37,7 @@ export const ChannelAutocomplete = React.memo(function ChannelAutocomplete({
activeItem?.scrollIntoView({ block: "nearest" });
}, [selectedIndex]);

if (suggestions.length === 0) {
if (!composerOwnsFocus || suggestions.length === 0) {
return null;
}

Expand All @@ -42,6 +48,7 @@ export const ChannelAutocomplete = React.memo(function ChannelAutocomplete({
position === "below" ? "top-full mt-1" : "bottom-full mb-1",
)}
>
{/* biome-ignore lint/a11y/noStaticElementInteractions: pointer-only guard, no behavior of its own — an unprevented mousedown here (scrollbar, padding ring) blurs the editor, and the focus gate above would unmount the overlay mid-press. */}
<div
className={cn(
"max-h-48 overflow-y-auto rounded-xl p-1",
Expand All @@ -51,6 +58,7 @@ export const ChannelAutocomplete = React.memo(function ChannelAutocomplete({
: "origin-bottom slide-in-from-bottom-1",
POPOVER_SURFACE_CLASS,
)}
onMouseDown={(event) => event.preventDefault()}
ref={listRef}
style={POPOVER_SHADOW_STYLE}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,10 @@ export function ComposerMentionButton({
data-testid="message-insert-mention"
disabled={disabled}
onClick={onOpen}
onMouseDown={onCaptureSelection}
onMouseDown={(event) => {
onCaptureSelection();
event.preventDefault();
}}
type="button"
>
<AtSign aria-hidden="true" className="h-4 w-4 shrink-0" />
Expand Down Expand Up @@ -303,6 +306,7 @@ export function ComposerMentionButton({
<button
className="shrink-0 rounded-md px-1.5 py-1 font-medium text-primary outline-hidden hover:bg-primary/10 focus-visible:ring-1 focus-visible:ring-ring"
onClick={onConfirmationTurnOff}
onMouseDown={(event) => event.preventDefault()}
type="button"
>
Turn off
Expand Down
Loading
Loading