Compare commits

..

4 Commits

Author SHA1 Message Date
agent_coder
d34b5f532f fix(#283 review r3): drop dead remap guard + use relative test import
F4: menu-items.layout.test.ts imports from './menu-items' (relative, no extension),
matching the sibling test files (was still the aliased '@/.../menu-items.ts').
F5: remove the dead 'candidate !== originalCandidate' clause from the remapped-candidate
filter — buildLayoutCandidates dedupes remaps against the original via Set, so the tail
after destructuring can never equal the original; the length gate is the only real
condition. Comment updated to state the dedup invariant instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-02 08:16:08 +03:00
agent_coder
0f4b03d89f fix(#283 review r2): gate remapped layout candidates against short-query over-match
This actually lands F1+F2 (round 1 pushed only the test rename by mistake).

F1: only the ORIGINAL query matches without length limits; remapped (wrong-layout)
candidates must be >= 3 chars before they can match, via a shared candidateMatchesItem
helper applied to both the item filter and the tie-break sort. Stops a 1-2 char ASCII
query from spuriously substring-matching Cyrillic searchTerms (/cy->сн no longer hits
'сноска', /b->и no longer hits 'примечание'), while keeping real wrong-layout commands
(/сщву->Code, /cyjcrf->Footnote), genuine short queries (/p, /h1) and Cyrillic terms
(/сноска->Footnote) working.
F2: reword the buildLayoutCandidates JSDoc (an ASCII query yields multiple candidates;
dedup only collapses when nothing is remappable).

Adds negative tests for /cy and /b.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-02 07:42:08 +03:00
agent_coder
d70b80c449 fix(#283 review): gate remapped layout candidates to avoid short-query over-match
F1: only the ORIGINAL query does full matching; remapped (wrong-layout) candidates
must be >= 3 chars and differ from the original before they can match (via a shared
candidateMatchesItem helper, applied to both the filter and the tie-break sort). This
stops a short remapped candidate from substring-matching the only cyrillic searchTerms
(/cy->сн, /b->и no longer surface Footnote) while keeping real wrong-layout commands
(/сщву->Code, /cyjcrf->Footnote) and genuine cyrillic terms (/сноска->Footnote) working.
F2: fix the buildLayoutCandidates JSDoc (an ascii query yields multiple candidates,
not a single-element set).
F3: rename the test to menu-items.layout.test.ts + relative import, per sibling convention.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-02 07:00:35 +03:00
agent_coder
5f02b7c80e fix(editor): match slash-menu commands typed in the wrong keyboard layout (closes #283)
Typing a command with the wrong layout (e.g. Russian ЙЦУКЕН -> /сщву for 'code')
matched nothing and collapsed the popup. Add ЙЦУКЕН<->QWERTY layout maps and a
buildLayoutCandidates(query) = [original, RU->EN, EN->RU]; getSuggestionItems now
matches an item if ANY candidate hits (fuzzy title / description / searchTerms),
and the tie-break sort is candidate-aware. Keeping the original among candidates
preserves genuine Cyrillic search terms (сноска -> Footnote). One-function change;
slash-command.ts allow() reuses it, so the popup-collapse is fixed transitively.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-02 05:44:58 +03:00
6 changed files with 183 additions and 128 deletions

View File

@@ -1,68 +0,0 @@
import { describe, it, expect, vi } from "vitest";
import { render } from "@testing-library/react";
// Covers the read-only render branch (PR #278): the language <Select> renders
// only when `editor.isEditable`; in read-only the copy button still shows.
// Mocks mirror the #146 structural harness (footnote-views.structure.test.tsx),
// except Select becomes a detectable node so we can assert its presence/absence.
vi.mock("@tiptap/react", () => ({
NodeViewWrapper: ({ children }: any) => <div>{children}</div>,
NodeViewContent: (props: any) => <div data-node-view-content="" {...props} />,
}));
vi.mock("react-i18next", () => ({
useTranslation: () => ({ t: (key: string) => key }),
}));
vi.mock("@mantine/core", () => ({
Group: ({ children }: any) => <div>{children}</div>,
Select: () => <div data-testid="language-select" />,
Tooltip: ({ children }: any) => <>{children}</>,
ActionIcon: ({ children, onClick }: any) => (
<button data-testid="copy-button" onClick={onClick}>
{children}
</button>
),
}));
vi.mock("@/components/common/copy-button", () => ({
CopyButton: ({ children }: any) => children({ copied: false, copy: () => {} }),
}));
vi.mock("@tabler/icons-react", () => ({
IconCheck: () => null,
IconCopy: () => null,
}));
vi.mock("@/features/editor/components/code-block/mermaid-view.tsx", () => ({
default: () => null,
}));
import CodeBlockView from "./code-block-view";
const makeProps = (isEditable: boolean) =>
({
node: { attrs: { language: "javascript" }, textContent: "", nodeSize: 1 },
editor: {
state: { selection: { from: 0, to: 0 } },
isEditable,
commands: {},
on: vi.fn(),
off: vi.fn(),
},
extension: {
options: { lowlight: { listLanguages: () => ["javascript", "python"] } },
},
getPos: () => 0,
updateAttributes: () => {},
deleteNode: () => {},
}) as any;
describe("CodeBlockView language selector visibility (#278)", () => {
it("renders the language selector when the editor is editable", () => {
const { queryByTestId } = render(<CodeBlockView {...makeProps(true)} />);
expect(queryByTestId("language-select")).not.toBeNull();
expect(queryByTestId("copy-button")).not.toBeNull();
});
it("hides the language selector in read-only but keeps the copy button", () => {
const { queryByTestId } = render(<CodeBlockView {...makeProps(false)} />);
expect(queryByTestId("language-select")).toBeNull();
expect(queryByTestId("copy-button")).not.toBeNull();
});
});

View File

@@ -50,10 +50,10 @@ export default function CodeBlockView(props: NodeViewProps) {
{/* #146: the editable <pre><code> (contentDOM) MUST come first in the DOM.
With the non-editable menu rendered before it, the browser's click
hit-testing snapped the caret up one line. Render content first; the
menu is rendered after it and floated into the top-right corner as an
absolute overlay (see `.menuGroup` in code-block.module.css, anchored
to the `position: relative` `.codeBlock` wrapper in code.css). It no
longer takes a full-width row above the code. The second #146
menu is rendered after it and lifted back above visually via flex
`order: -1` (the `.codeBlock` wrapper is a flex column — see
code-block.module.css). It stays fully in flow as a full-width row
above the code: no overlay/absolute positioning. The second #146
mitigation lives in editor-paste-handler.tsx (reflowAfterPaste). */}
<pre
spellCheck="false"
@@ -67,23 +67,22 @@ export default function CodeBlockView(props: NodeViewProps) {
<NodeViewContent as="code" className={`language-${language}`} />
</pre>
<Group contentEditable={false} className={classes.menuGroup}>
{/* In read-only (published) there is no language selector at all —
only the copy button. When editable the selector is hidden until
the block is hovered/focused (or its dropdown is open) via the
`.languageSelect` class (see code-block.module.css). */}
{editor.isEditable && (
<Select
placeholder="auto"
checkIconPosition="right"
data={extension.options.lowlight.listLanguages().sort()}
value={languageValue}
onChange={changeLanguage}
searchable
style={{ maxWidth: "130px" }}
classNames={{ root: classes.languageSelect, input: classes.selectInput }}
/>
)}
<Group
justify="flex-end"
contentEditable={false}
className={classes.menuGroup}
>
<Select
placeholder="auto"
checkIconPosition="right"
data={extension.options.lowlight.listLanguages().sort()}
value={languageValue}
onChange={changeLanguage}
searchable
style={{ maxWidth: "130px" }}
classNames={{ input: classes.selectInput }}
disabled={!editor.isEditable}
/>
<CopyButton value={node?.textContent} timeout={2000}>
{({ copied, copy }) => (

View File

@@ -17,37 +17,15 @@
justify-content: center;
}
/* #146: the menu follows the <pre> in the DOM (so the editable contentDOM is
FIRST and click hit-testing is correct). Instead of sitting in-flow, it is
floated into the top-right corner as an absolute overlay anchored to the
`position: relative` .codeBlock wrapper (see code.css), so it no longer
takes a full-width row above the code. The Mantine dropdown is portaled, so
it is never clipped by the overlay. */
/* #146: the menu now follows the <pre> in the DOM (so the editable contentDOM is
FIRST and click hit-testing is correct). Lift it back ABOVE the code visually
with flex `order` — the .codeBlock wrapper is a flex column (see code.css) —
so the menu still reads as a row above the code, exactly as before, without
sitting in-flow before the contentDOM. */
.menuGroup {
position: absolute;
top: 8px;
right: 8px;
z-index: 1;
gap: 4px;
order: -1;
@media print {
display: none;
}
}
/* The language selector is hidden until the block is hovered, or the selector
itself is focused / its dropdown is open. It keeps its width in the flex
Group (only opacity toggles) so the copy button never jumps, and
`pointer-events: none` while hidden lets clicks fall through to the code.
`.codeBlock` is the global NodeViewWrapper class → use :global(). */
.languageSelect {
opacity: 0;
pointer-events: none;
transition: opacity 150ms ease;
}
:global(.codeBlock):hover .languageSelect,
.languageSelect:focus-within {
opacity: 1;
pointer-events: auto;
}

View File

@@ -0,0 +1,75 @@
import { describe, it, expect } from "vitest";
import {
buildLayoutCandidates,
getSuggestionItems,
} from "./menu-items";
/**
* `buildLayoutCandidates` maps a slash query across physical keyboard layouts
* (RU ЙЦУКЕН <-> US QWERTY) so the menu matches Latin item titles/terms even
* when typed with the wrong layout active, while keeping the original query so
* genuine Cyrillic search terms still match. See bug #283.
*/
describe("buildLayoutCandidates", () => {
it("remaps a RU-layout query to its US-QWERTY equivalent (сщву -> code)", () => {
expect(buildLayoutCandidates("сщву")).toContain("code");
});
it("remaps a US-layout query to its RU-ЙЦУКЕН equivalent (cyjcrf -> сноска)", () => {
expect(buildLayoutCandidates("cyjcrf")).toContain("сноска");
});
it("always includes the original query", () => {
expect(buildLayoutCandidates("сщву")).toContain("сщву");
expect(buildLayoutCandidates("cyjcrf")).toContain("cyjcrf");
expect(buildLayoutCandidates("сноска")).toContain("сноска");
});
it("leaves a query with no mappable keys as a single-element set", () => {
// Digits are on neither layout map, so both remaps are no-ops and de-dup
// back to one entry.
expect(buildLayoutCandidates("123")).toEqual(["123"]);
});
});
/** Helper: flatten grouped suggestion items to a flat list of titles. */
const titles = (groups: ReturnType<typeof getSuggestionItems>): string[] =>
Object.values(groups).flatMap((items) => items.map((i) => i.title));
describe("getSuggestionItems layout-aware matching", () => {
it("finds Code when 'code' is typed in RU layout (/сщву)", () => {
expect(titles(getSuggestionItems({ query: "сщву" }))).toContain("Code");
});
it("still finds Code for the plain /code query", () => {
expect(titles(getSuggestionItems({ query: "code" }))).toContain("Code");
});
it("still matches genuine Cyrillic search terms (/сноска -> Footnote)", () => {
expect(titles(getSuggestionItems({ query: "сноска" }))).toContain(
"Footnote",
);
});
it("finds Footnote when 'сноска' is typed in EN layout (/cyjcrf)", () => {
expect(titles(getSuggestionItems({ query: "cyjcrf" }))).toContain(
"Footnote",
);
});
it("does not surface Footnote for a short wrong-layout query (/cy)", () => {
// "cy" EN->RU remaps to "сн", a substring of the "сноска" searchTerm, but
// the gate blocks it because the remapped candidate is < 3 chars.
expect(titles(getSuggestionItems({ query: "cy" }))).not.toContain(
"Footnote",
);
});
it("does not surface Footnote for a single-char wrong-layout query (/b)", () => {
// "b" EN->RU remaps to "и", a substring of the "примечание" searchTerm, but
// the gate blocks it because the remapped candidate is < 3 chars.
expect(titles(getSuggestionItems({ query: "b" }))).not.toContain(
"Footnote",
);
});
});

View File

@@ -35,6 +35,7 @@ import { PAGE_EMBED_PICKER_EVENT } from "@/features/editor/components/page-embed
import {
CommandProps,
SlashMenuGroupedItemsType,
SlashMenuItemType,
} from "@/features/editor/components/slash-menu/types";
import { uploadImageAction } from "@/features/editor/components/image/upload-image-action.tsx";
import { uploadVideoAction } from "@/features/editor/components/video/upload-video-action.tsx";
@@ -835,6 +836,49 @@ export function isHtmlEmbedFeatureEnabled(): boolean {
}
}
// Russian ЙЦУКЕН -> US QWERTY by physical key position (lowercase; callers
// lowercase first). Lets the slash menu match Latin item titles/terms even when
// a command is typed with the wrong keyboard layout active (e.g. "/сщву" while
// ЙЦУКЕН is on physically types the same keys as "/code").
const RU_TO_EN_LAYOUT: Record<string, string> = {
й: "q", ц: "w", у: "e", к: "r", е: "t", н: "y", г: "u", ш: "i", щ: "o",
з: "p", х: "[", ъ: "]",
ф: "a", ы: "s", в: "d", а: "f", п: "g", р: "h", о: "j", л: "k", д: "l",
ж: ";", э: "'",
я: "z", ч: "x", с: "c", м: "v", и: "b", т: "n", ь: "m", б: ",", ю: ".",
ё: "`",
};
// Inverse map: US QWERTY -> Russian ЙЦУКЕН by physical key position. Handles the
// mirror case (e.g. "cyjcrf" typed with EN layout on == "сноска" == Footnote).
const EN_TO_RU_LAYOUT: Record<string, string> = Object.fromEntries(
Object.entries(RU_TO_EN_LAYOUT).map(([ru, en]) => [en, ru]),
);
function translitByLayout(text: string, map: Record<string, string>): string {
let out = "";
for (const ch of text) out += map[ch] ?? ch;
return out;
}
/**
* Build the list of search strings to try for a given query: the original
* query first, followed by its RU->EN and EN->RU physical-layout remappings.
* Keeping the original first preserves genuine Cyrillic search terms (e.g.
* "сноска"/"примечание" for Footnote) and lets callers treat the original
* differently from the remapped candidates. De-duplication only collapses the
* list to one element when nothing is remappable (e.g. digits/spaces), so a
* typical ASCII query still yields multiple candidates.
*/
export function buildLayoutCandidates(search: string): string[] {
return [
...new Set([
search,
translitByLayout(search, RU_TO_EN_LAYOUT),
translitByLayout(search, EN_TO_RU_LAYOUT),
]),
];
}
export const getSuggestionItems = ({
query,
excludeItems,
@@ -843,6 +887,18 @@ export const getSuggestionItems = ({
excludeItems?: Set<string>;
}): SlashMenuGroupedItemsType => {
const search = query.toLowerCase();
const candidates = buildLayoutCandidates(search);
// Only the original query is allowed to match via a short substring. Remapped
// (wrong-layout) candidates must be at least REMAP_MIN_LEN chars before they
// can match, so a 1-2 char ASCII query does not spuriously substring-match
// unrelated Cyrillic search terms (e.g. "/cy" -> "сн" hitting "сноска",
// "/b" -> "и" hitting "примечание"). buildLayoutCandidates already dedupes
// the remaps against the original, so candidates[0] is the original query.
const REMAP_MIN_LEN = 3;
const [originalCandidate, ...remapped] = candidates;
const remappedCandidates = remapped.filter(
(candidate) => candidate.length >= REMAP_MIN_LEN,
);
const filteredGroups: SlashMenuGroupedItemsType = {};
const htmlEmbedFeatureEnabled = isHtmlEmbedFeatureEnabled();
@@ -856,24 +912,42 @@ export const getSuggestionItems = ({
return false;
};
const candidateMatchesItem = (
candidate: string,
item: SlashMenuItemType,
description: string,
) =>
fuzzyMatch(candidate, item.title) ||
description.includes(candidate) ||
(item.searchTerms != null &&
item.searchTerms.some((term: string) => term.includes(candidate)));
for (const [group, items] of Object.entries(CommandGroups)) {
const filteredItems = items.filter((item) => {
if (excludeItems?.has(item.title)) return false;
// Hide the HTML embed item unless the workspace master toggle is ON.
if (item.requiresHtmlEmbedFeature && !htmlEmbedFeatureEnabled)
return false;
const description = item.description.toLowerCase();
return (
fuzzyMatch(search, item.title) ||
item.description.toLowerCase().includes(search) ||
(item.searchTerms &&
item.searchTerms.some((term: string) => term.includes(search)))
candidateMatchesItem(originalCandidate, item, description) ||
remappedCandidates.some((candidate) =>
candidateMatchesItem(candidate, item, description),
)
);
});
if (filteredItems.length) {
const titleMatchesAnyCandidate = (title: string) => {
const lower = title.toLowerCase();
return (
lower.includes(originalCandidate) ||
remappedCandidates.some((candidate) => lower.includes(candidate))
);
};
filteredGroups[group] = filteredItems.sort((a, b) => {
const aTitle = a.title.toLowerCase().includes(search) ? 0 : 1;
const bTitle = b.title.toLowerCase().includes(search) ? 0 : 1;
const aTitle = titleMatchesAnyCandidate(a.title) ? 0 : 1;
const bTitle = titleMatchesAnyCandidate(b.title) ? 0 : 1;
return aTitle - bTitle;
});
}

View File

@@ -1,12 +1,9 @@
.ProseMirror {
.codeBlock {
/* #146: flex column keeps the editable <pre> (first in the DOM so click
hit-testing is correct) laid out above any Mermaid diagram. `position:
relative` anchors the control panel, which is floated into the top-right
corner as an absolute overlay (see `.menuGroup` in code-block.module.css). */
/* #146: flex column so the menu (rendered AFTER <pre> in the DOM, so the
editable contentDOM is first) is lifted back above the code via `order`. */
display: flex;
flex-direction: column;
position: relative;
padding: 4px;
border-radius: var(--mantine-radius-default);
background-color: light-dark(var(--mantine-color-gray-0), var(--mantine-color-dark-8));