mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): make ⌘K open the composer link editor for selections (#1644)
Signed-off-by: Aaron Goldsmith <aarong@squareup.com>
This commit is contained in:
@@ -46,6 +46,7 @@ export default defineConfig({
|
||||
"**/composer-image-draw.spec.ts",
|
||||
"**/video-attachment.spec.ts",
|
||||
"**/spoiler.spec.ts",
|
||||
"**/composer-link-shortcut.spec.ts",
|
||||
"**/composer-tooltip-dismiss.spec.ts",
|
||||
"**/mentions.spec.ts",
|
||||
"**/relay-reconnect.spec.ts",
|
||||
|
||||
@@ -255,7 +255,10 @@ const overrides = new Map([
|
||||
// + mount-only useEffect for the Drafts-panel "Send message" confirm-dialog
|
||||
// flow. Load-bearing feature growth; queued to split with the rest of this
|
||||
// list.
|
||||
["src/features/messages/ui/MessageComposer.tsx", 1033],
|
||||
// +3: onLinkShortcutRef wiring (ref decl + editor option + assignment) for
|
||||
// the ⌘K link-editor shortcut, mirroring the existing onEditLinkRef
|
||||
// pattern. Queued to split with the rest of this list.
|
||||
["src/features/messages/ui/MessageComposer.tsx", 1036],
|
||||
]);
|
||||
|
||||
await runFileSizeCheck({
|
||||
|
||||
@@ -528,7 +528,16 @@ export function AppShell() {
|
||||
}
|
||||
|
||||
function handleKeyDown(event: KeyboardEvent) {
|
||||
if (!hasPrimaryShortcutModifier(event) || event.altKey) {
|
||||
if (!hasPrimaryShortcutModifier(event) || event.altKey || event.repeat) {
|
||||
return;
|
||||
}
|
||||
|
||||
// A focused surface may claim the shortcut first — e.g. the composer
|
||||
// consumes ⌘K to open the link editor when text is selected. Its
|
||||
// element-level handler runs before this window-level bubble listener
|
||||
// and calls `preventDefault()`; respect that instead of also opening
|
||||
// the global dialog.
|
||||
if (event.defaultPrevented) {
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
@@ -89,6 +89,7 @@ export function ForumComposer({
|
||||
const onLinkSelectionChangeRef = React.useRef<
|
||||
((info: LinkSelectionInfo | null) => void) | null
|
||||
>(null);
|
||||
const onLinkShortcutRef = React.useRef<(() => boolean) | null>(null);
|
||||
|
||||
const richText = useRichTextEditor({
|
||||
placeholder,
|
||||
@@ -99,6 +100,7 @@ export function ForumComposer({
|
||||
isAutocompleteOpen: isAutocompleteOpenRef,
|
||||
onEditLink: (info) => onEditLinkRef.current?.(info),
|
||||
onLinkSelectionChange: (info) => onLinkSelectionChangeRef.current?.(info),
|
||||
onLinkShortcut: () => onLinkShortcutRef.current?.() ?? false,
|
||||
onUpdate: ({ cursor, text }) => {
|
||||
const markdown = richText.getMarkdown();
|
||||
setContent(markdown);
|
||||
@@ -112,6 +114,7 @@ export function ForumComposer({
|
||||
const linkEditor = useLinkEditor(richText);
|
||||
onEditLinkRef.current = linkEditor.openFromClick;
|
||||
onLinkSelectionChangeRef.current = linkEditor.showFromCursor;
|
||||
onLinkShortcutRef.current = linkEditor.openFromShortcut;
|
||||
|
||||
// ── Mention / channel autocomplete insertion ────────────────────────
|
||||
// Native ProseMirror transactions — no markdown round-trip.
|
||||
|
||||
@@ -58,6 +58,9 @@ type LinkCardState = {
|
||||
* Returns:
|
||||
* - `openFromToolbar` — wire to the formatting toolbar's link button. Seeds
|
||||
* the dialog from the current selection (existing link or selected text).
|
||||
* - `openFromShortcut` — wire to the editor's ⌘K handler. Opens the dialog
|
||||
* only when a selection or caret-adjacent link exists; returns whether it
|
||||
* consumed the shortcut.
|
||||
* - `openFromClick` — wire to `useRichTextEditor`'s `onEditLink`. Moves the
|
||||
* clicked link into the hover-card state.
|
||||
* - `showFromCursor` — wire to cursor/selection updates to show the same card
|
||||
@@ -179,6 +182,20 @@ export function useLinkEditor(richText: UseRichTextEditorResult) {
|
||||
});
|
||||
}, [getLinkSelectionInfo, openDialogFromInfo]);
|
||||
|
||||
/**
|
||||
* ⌘K/Ctrl+K handler. Opens the link dialog only when the shortcut applies —
|
||||
* text is selected or the caret sits inside an existing link — and reports
|
||||
* whether it did, so the caller can leave the keystroke to the app-wide
|
||||
* quick-search binding otherwise.
|
||||
*/
|
||||
const openFromShortcut = React.useCallback((): boolean => {
|
||||
const info = getLinkSelectionInfo();
|
||||
if (!info) return false;
|
||||
setCardState(null);
|
||||
openDialogFromInfo(info);
|
||||
return true;
|
||||
}, [getLinkSelectionInfo, openDialogFromInfo]);
|
||||
|
||||
const close = React.useCallback(() => setDraft(null), []);
|
||||
|
||||
const closeCard = React.useCallback(() => setCardState(null), []);
|
||||
@@ -431,6 +448,7 @@ export function useLinkEditor(richText: UseRichTextEditorResult) {
|
||||
|
||||
return {
|
||||
openFromToolbar,
|
||||
openFromShortcut,
|
||||
openFromClick,
|
||||
showFromCursor: showCard,
|
||||
focusCardFirstControl,
|
||||
|
||||
@@ -8,7 +8,10 @@ import Link from "@tiptap/extension-link";
|
||||
import { Extension, type KeyboardShortcutCommand } from "@tiptap/core";
|
||||
import { Plugin, Selection, TextSelection } from "@tiptap/pm/state";
|
||||
|
||||
import { isMacPlatform } from "@/shared/lib/platform";
|
||||
import {
|
||||
hasPrimaryShortcutModifier,
|
||||
isMacPlatform,
|
||||
} from "@/shared/lib/platform";
|
||||
import type { CustomEmoji } from "@/shared/lib/remarkCustomEmoji";
|
||||
|
||||
import { resolveLinkAt, type LinkSelectionInfo } from "./resolveLinkAt";
|
||||
@@ -92,6 +95,15 @@ export type RichTextEditorOptions = {
|
||||
* this for link affordances that follow keyboard cursor movement.
|
||||
*/
|
||||
onLinkSelectionChange?: (info: LinkSelectionInfo | null) => void;
|
||||
/**
|
||||
* Called on ⌘K/Ctrl+K while the editor has focus. The owner should open
|
||||
* the link-edit modal when the shortcut applies (text is selected, or the
|
||||
* caret sits inside an existing link) and return `true` to consume the
|
||||
* keystroke. Return `false` to let the event fall through to the global
|
||||
* quick-search shortcut — a bare caret in the composer must not hijack
|
||||
* app-wide ⌘K muscle memory.
|
||||
*/
|
||||
onLinkShortcut?: () => boolean;
|
||||
};
|
||||
|
||||
const PASTED_LINK_AT_END_RE =
|
||||
@@ -173,6 +185,7 @@ export function useRichTextEditor({
|
||||
isAutocompleteOpen,
|
||||
onEditLink,
|
||||
onLinkSelectionChange,
|
||||
onLinkShortcut,
|
||||
}: RichTextEditorOptions) {
|
||||
const onUpdateRef = React.useRef(onUpdate);
|
||||
onUpdateRef.current = onUpdate;
|
||||
@@ -189,6 +202,9 @@ export function useRichTextEditor({
|
||||
const onLinkSelectionChangeRef = React.useRef(onLinkSelectionChange);
|
||||
onLinkSelectionChangeRef.current = onLinkSelectionChange;
|
||||
|
||||
const onLinkShortcutRef = React.useRef(onLinkShortcut);
|
||||
onLinkShortcutRef.current = onLinkShortcut;
|
||||
|
||||
const placeholderRef = React.useRef(placeholder);
|
||||
placeholderRef.current = placeholder;
|
||||
|
||||
@@ -444,6 +460,30 @@ export function useRichTextEditor({
|
||||
// command/caret logic, fires regardless of selection state, and works
|
||||
// the same across browser engines. Returning `true` consumes the key.
|
||||
handleKeyDown: (view, event) => {
|
||||
// ⌘K / Ctrl+K → link editor. The formatting toolbar has always
|
||||
// advertised this shortcut on its link button; bind it here so it
|
||||
// actually works. Kept alongside the ArrowUp handling below rather
|
||||
// than in a keymap extension so the modifier discrimination is
|
||||
// explicit. Only *conditionally* consumed: the owner returns `true`
|
||||
// only when the shortcut applies (selection or caret-on-link), so a
|
||||
// bare caret still falls through to the app-wide quick-search
|
||||
// binding in `AppShell`. Returning `true` makes ProseMirror call
|
||||
// `preventDefault()`, which the AppShell window listener respects
|
||||
// via `event.defaultPrevented`.
|
||||
if (
|
||||
event.key.toLowerCase() === "k" &&
|
||||
hasPrimaryShortcutModifier(event) &&
|
||||
!event.shiftKey &&
|
||||
!event.altKey &&
|
||||
// Ignore held-key auto-repeat (the first press already opened the
|
||||
// dialog and moved focus into it) and mid-IME composition, where
|
||||
// the selection may span uncommitted composition text.
|
||||
!event.repeat &&
|
||||
!event.isComposing
|
||||
) {
|
||||
return onLinkShortcutRef.current?.() ?? false;
|
||||
}
|
||||
|
||||
if (event.key !== "ArrowUp") return false;
|
||||
// Respect the same guards as before: no modifiers (let ⌥↑/⇧↑/etc.
|
||||
// through), autocomplete closed, a handler exists, and the composer
|
||||
|
||||
@@ -276,6 +276,7 @@ function MessageComposerImpl({
|
||||
const onLinkSelectionChangeRef = React.useRef<
|
||||
((info: LinkSelectionInfo | null) => void) | null
|
||||
>(null);
|
||||
const onLinkShortcutRef = React.useRef<(() => boolean) | null>(null);
|
||||
|
||||
const scrollComposerToBottom = React.useCallback(() => {
|
||||
window.requestAnimationFrame(() => {
|
||||
@@ -310,6 +311,7 @@ function MessageComposerImpl({
|
||||
isAutocompleteOpen: isAutocompleteOpenRef,
|
||||
onEditLink: (info) => onEditLinkRef.current?.(info),
|
||||
onLinkSelectionChange: (info) => onLinkSelectionChangeRef.current?.(info),
|
||||
onLinkShortcut: () => onLinkShortcutRef.current?.() ?? false,
|
||||
onUpdate: ({ cursor, text }) => {
|
||||
setComposerContentFromText(text);
|
||||
|
||||
@@ -331,6 +333,7 @@ function MessageComposerImpl({
|
||||
};
|
||||
onEditLinkRef.current = linkEditor.openFromClick;
|
||||
onLinkSelectionChangeRef.current = linkEditor.showFromCursor;
|
||||
onLinkShortcutRef.current = linkEditor.openFromShortcut;
|
||||
useComposerSpoilerParticles(richText.editor, composerScrollRef);
|
||||
|
||||
const mentionSendFlow = useMentionSendFlow({
|
||||
|
||||
@@ -218,7 +218,8 @@ export const KEYBOARD_SHORTCUTS: KeyboardShortcut[] = [
|
||||
{
|
||||
id: "format-link",
|
||||
label: "Insert link",
|
||||
description: "Insert or edit a link in the composer",
|
||||
description:
|
||||
"Link the selected composer text, or edit the link under the caret",
|
||||
keys: "⌘K",
|
||||
keysWindows: "Ctrl+K",
|
||||
category: "Formatting",
|
||||
|
||||
@@ -0,0 +1,138 @@
|
||||
import { expect, test } from "@playwright/test";
|
||||
import type { Page } from "@playwright/test";
|
||||
|
||||
import { installMockBridge } from "../helpers/bridge";
|
||||
|
||||
// ⌘K / Ctrl+K behaviour around the composer:
|
||||
// - With composer text selected → open the link-edit dialog (the shortcut the
|
||||
// formatting toolbar has always advertised on its link button).
|
||||
// - With a caret inside an existing composer link → open the same dialog
|
||||
// seeded with that link.
|
||||
// - With an empty caret in the composer (no selection, no link) → fall
|
||||
// through to the app-wide quick-search dialog.
|
||||
// - With focus outside the composer → quick search, unchanged.
|
||||
|
||||
async function openGeneral(page: Page) {
|
||||
await page.goto("/");
|
||||
await page.getByTestId("channel-general").click();
|
||||
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
||||
}
|
||||
|
||||
test("⌘K with selected composer text opens the add-link dialog", async ({
|
||||
page,
|
||||
}) => {
|
||||
await installMockBridge(page);
|
||||
await openGeneral(page);
|
||||
|
||||
const input = page.getByTestId("message-input");
|
||||
await input.click();
|
||||
await input.pressSequentially("check out this link");
|
||||
await page.keyboard.press("ControlOrMeta+a");
|
||||
await page.keyboard.press("ControlOrMeta+k");
|
||||
|
||||
const dialog = page.getByRole("dialog", { name: "Add link" });
|
||||
await expect(dialog).toBeVisible();
|
||||
// Seeded with the selected text as the display value.
|
||||
await expect(dialog.getByLabel("Display text")).toHaveValue(
|
||||
"check out this link",
|
||||
);
|
||||
// Quick search must NOT have opened.
|
||||
await expect(page.getByTestId("search-dialog-input")).toHaveCount(0);
|
||||
});
|
||||
|
||||
test("⌘K with caret inside an existing link opens the edit-link dialog", async ({
|
||||
page,
|
||||
}) => {
|
||||
await installMockBridge(page);
|
||||
await openGeneral(page);
|
||||
|
||||
const input = page.getByTestId("message-input");
|
||||
await input.click();
|
||||
// Create a real link through the ⌘K flow first.
|
||||
await input.pressSequentially("docs");
|
||||
await page.keyboard.press("ControlOrMeta+a");
|
||||
await page.keyboard.press("ControlOrMeta+k");
|
||||
const addDialog = page.getByRole("dialog", { name: "Add link" });
|
||||
await expect(addDialog).toBeVisible();
|
||||
await addDialog.getByLabel("URL").fill("https://example.com");
|
||||
await addDialog.getByRole("button", { name: "Save" }).click();
|
||||
await expect(addDialog).toHaveCount(0);
|
||||
await expect(input.locator('a[href="https://example.com"]')).toHaveText(
|
||||
"docs",
|
||||
);
|
||||
|
||||
// Click into the linked text to place the caret inside it, then re-trigger
|
||||
// the shortcut. (The click also surfaces the composer link hover card —
|
||||
// ⌘K must open the full dialog from that state.)
|
||||
await input.locator('a[href="https://example.com"]').click();
|
||||
await page.keyboard.press("ControlOrMeta+k");
|
||||
|
||||
const editDialog = page.getByRole("dialog", { name: "Edit link" });
|
||||
await expect(editDialog).toBeVisible();
|
||||
await expect(editDialog.getByLabel("URL")).toHaveValue("https://example.com");
|
||||
await expect(page.getByTestId("search-dialog-input")).toHaveCount(0);
|
||||
});
|
||||
|
||||
test("⌘K with an empty composer caret still opens quick search", async ({
|
||||
page,
|
||||
}) => {
|
||||
await installMockBridge(page);
|
||||
await openGeneral(page);
|
||||
|
||||
const input = page.getByTestId("message-input");
|
||||
await input.click();
|
||||
await page.keyboard.press("ControlOrMeta+k");
|
||||
|
||||
await expect(page.getByTestId("search-dialog-input")).toBeVisible();
|
||||
await expect(page.getByRole("dialog", { name: "Add link" })).toHaveCount(0);
|
||||
});
|
||||
|
||||
test("⌘K with unselected composer text (caret only) opens quick search", async ({
|
||||
page,
|
||||
}) => {
|
||||
await installMockBridge(page);
|
||||
await openGeneral(page);
|
||||
|
||||
const input = page.getByTestId("message-input");
|
||||
await input.click();
|
||||
await input.pressSequentially("draft in progress");
|
||||
await page.keyboard.press("ControlOrMeta+k");
|
||||
|
||||
await expect(page.getByTestId("search-dialog-input")).toBeVisible();
|
||||
await expect(page.getByRole("dialog", { name: "Add link" })).toHaveCount(0);
|
||||
});
|
||||
|
||||
test("macOS plain Ctrl+K still kill-lines in the composer", async ({
|
||||
page,
|
||||
}) => {
|
||||
// Regression guard for the Emacs-style Ctrl-K binding: on macOS the
|
||||
// primary-modifier check must reject Control so `macEmacsTextShortcuts`
|
||||
// keeps kill-line, and neither the link dialog nor quick search may open.
|
||||
test.skip(process.platform !== "darwin", "mac-only Emacs binding");
|
||||
|
||||
await installMockBridge(page);
|
||||
await openGeneral(page);
|
||||
|
||||
const input = page.getByTestId("message-input");
|
||||
await input.click();
|
||||
await input.pressSequentially("kill this line");
|
||||
// Emacs Ctrl-A → start of line (ProseMirror handles this natively on mac;
|
||||
// "Home" is not reliable in headless Chromium).
|
||||
await page.keyboard.press("Control+a");
|
||||
await page.keyboard.press("Control+k");
|
||||
|
||||
await expect(input).not.toContainText("kill this line");
|
||||
await expect(page.getByRole("dialog", { name: "Add link" })).toHaveCount(0);
|
||||
await expect(page.getByTestId("search-dialog-input")).toHaveCount(0);
|
||||
});
|
||||
|
||||
test("⌘K outside the composer opens quick search", async ({ page }) => {
|
||||
await installMockBridge(page);
|
||||
await openGeneral(page);
|
||||
|
||||
// Focus is on the page body — not the composer.
|
||||
await page.getByTestId("chat-title").click();
|
||||
await page.keyboard.press("ControlOrMeta+k");
|
||||
|
||||
await expect(page.getByTestId("search-dialog-input")).toBeVisible();
|
||||
});
|
||||
Reference in New Issue
Block a user