diff --git a/extensions/src/platform-scripture-editor/src/_usj-nodes.scss b/extensions/src/platform-scripture-editor/src/_usj-nodes.scss index 6d50fb19f28..06745e99ab1 100644 --- a/extensions/src/platform-scripture-editor/src/_usj-nodes.scss +++ b/extensions/src/platform-scripture-editor/src/_usj-nodes.scss @@ -6,6 +6,8 @@ demo copy's d95907eed30 sync — the two vendored copies must not diverge on those rules). When re-syncing, see the "Indent compensation" comment in the gutter section below for the invariant usj-nodes-scss-coverage.test.ts enforces on this file and the demo copy. + `.psc-para-marker-selected` is this repo's own rule: the library styles that class in its + editor.css, which this repo does not load, and the colours here are theme tokens. The source's four `local()`-only Charis SIL @font-face rules are dropped. */ /* stylelint-disable */ @@ -3012,6 +3014,48 @@ span.read img { right: calc(-1 * (var(--psc-gutter-width) - 0.5em) - var(--para-indent)); } +/* Paragraph-marker selection (paragraph-structure view). The editor puts `psc-para-marker-selected` + on the paragraph whose gutter marker is the selection. Selection gets a channel of its own, as in + adr-list-selection-on-a-dedicated-visual-channel (a 4px leading bar, never the active-text focus + box, which owns this element's ::after, and no pseudo-element), but diverges from that ADR in two + ways. It adds a row fill, which the ADR rejects for comment cards: those backgrounds carry status, + whereas a paragraph row's background carries none. And the bar is a box-shadow rather than a + logical border-inline-start, because the gutter lies outside the paragraph's box — the glyph is + absolutely positioned into .editor-input's padding — where no border can reach; box-shadow has no + logical form, hence the explicit RTL rule below. Both layers are outer box-shadows offset toward + inline-start by the gutter width plus the paragraph's own indent compensation; the fill layer + stops 4px short, which is the bar. `--foreground` is the bar token by measured contrast (>= 3:1 + against --background and --accent in every built-in theme), pinned by + paragraph-marker-selection-contrast.test.ts. */ +.psc-gutter-markers .psc-para-marker-selected { + --psc-para-marker-selection-reach: calc(var(--psc-gutter-width) + var(--para-indent, 0px)); + background-color: var(--accent); + box-shadow: + calc(4px - var(--psc-para-marker-selection-reach)) 0 0 0 var(--accent), + calc(-1 * var(--psc-para-marker-selection-reach)) 0 0 0 var(--foreground); +} + +.psc-gutter-markers[dir='rtl'] .psc-para-marker-selected { + box-shadow: + calc(var(--psc-para-marker-selection-reach) - 4px) 0 0 0 var(--accent), + var(--psc-para-marker-selection-reach) 0 0 0 var(--foreground); +} + +.psc-gutter-markers .psc-para-marker-selected > .marker:not(.verse):not(.chapter):first-child { + color: var(--accent-foreground); + font-weight: 700; +} + +/* Forced colors (e.g. Windows high contrast) drop box-shadow and replace the --accent fill with the + system background, which would leave only the bold glyph to mark the selection. An outline in the + system Highlight colour survives there and, drawn around the whole paragraph box, needs no + direction-specific rule. */ +@media (forced-colors: active) { + .psc-gutter-markers .psc-para-marker-selected { + outline: 2px solid Highlight; + } +} + /* ── Active text focus box ──────────────────────────────────────────────────── Applied via .psc-active-focus when viewOptions.hasActiveTextFocusBox is true. Shows an outline box around the verse range under the cursor. When used diff --git a/extensions/src/platform-scripture-editor/src/character-marker-control/character-marker-control.component.tsx b/extensions/src/platform-scripture-editor/src/character-marker-control/character-marker-control.component.tsx index 33ed7738812..25c4d40a128 100644 --- a/extensions/src/platform-scripture-editor/src/character-marker-control/character-marker-control.component.tsx +++ b/extensions/src/platform-scripture-editor/src/character-marker-control/character-marker-control.component.tsx @@ -26,6 +26,7 @@ import { SEARCH_PLACEHOLDER_KEY, SYNC_BLOCKED_KEY, } from './character-marker-control.const'; +import { wrapMarkerMenuItemsWithClose } from '../marker-menu.utils'; export { CHARACTER_MARKER_CONTROL_STRING_KEYS } from './character-marker-control.const'; @@ -204,18 +205,9 @@ export function CharacterMarkerControl({ }, [onClose]); // This is a single-select control, so picking a marker must close the menu — and closing is also - // what returns focus to the editor, via `onClose`. `MarkerMenu` wires `onSelect` straight to - // `item.action` and knows nothing about its host's open state, so the host wraps each action - // rather than the menu closing itself. + // what returns focus to the editor, via `onClose`. const closingMarkerMenuItems = useMemo( - () => - markerMenuItems.map((item) => ({ - ...item, - action: () => { - item.action(); - closeMenu(); - }, - })), + () => wrapMarkerMenuItemsWithClose(markerMenuItems, closeMenu), [markerMenuItems, closeMenu], ); diff --git a/extensions/src/platform-scripture-editor/src/marker-menu.utils.test.ts b/extensions/src/platform-scripture-editor/src/marker-menu.utils.test.ts new file mode 100644 index 00000000000..71f762b5620 --- /dev/null +++ b/extensions/src/platform-scripture-editor/src/marker-menu.utils.test.ts @@ -0,0 +1,47 @@ +import { describe, it, expect, vi } from 'vitest'; +import { MarkerMenuItem } from 'platform-bible-react'; +import { wrapMarkerMenuItemsWithClose } from './marker-menu.utils'; + +function makeItem(marker: string, action = vi.fn()): MarkerMenuItem { + return { marker, title: `Title ${marker}`, action }; +} + +describe('wrapMarkerMenuItemsWithClose', () => { + it("runs the picked item's action, then closes the menu", () => { + const calls: string[] = []; + const items = [ + makeItem( + 'p', + vi.fn(() => calls.push('action')), + ), + ]; + + const [wrapped] = wrapMarkerMenuItemsWithClose(items, () => calls.push('close')); + wrapped.action(); + + expect(calls).toEqual(['action', 'close']); + }); + + it('runs only the picked item and closes once', () => { + const first = makeItem('p'); + const second = makeItem('q1'); + const close = vi.fn(); + + wrapMarkerMenuItemsWithClose([first, second], close)[1].action(); + + expect(second.action).toHaveBeenCalledTimes(1); + expect(first.action).not.toHaveBeenCalled(); + expect(close).toHaveBeenCalledTimes(1); + }); + + it('keeps every other field and leaves the input items unchanged', () => { + const original = makeItem('s1'); + const originalAction = original.action; + + const [wrapped] = wrapMarkerMenuItemsWithClose([original], vi.fn()); + + expect(wrapped).toEqual({ ...original, action: expect.any(Function) }); + expect(wrapped.action).not.toBe(originalAction); + expect(original.action).toBe(originalAction); + }); +}); diff --git a/extensions/src/platform-scripture-editor/src/marker-menu.utils.ts b/extensions/src/platform-scripture-editor/src/marker-menu.utils.ts new file mode 100644 index 00000000000..46948d0a978 --- /dev/null +++ b/extensions/src/platform-scripture-editor/src/marker-menu.utils.ts @@ -0,0 +1,25 @@ +import { MarkerMenuItem } from 'platform-bible-react'; + +/** + * Wraps each item's action so that picking it also closes the host's menu. + * + * A single-select marker control must close on pick, but `MarkerMenu` wires `onSelect` straight to + * `item.action` and knows nothing about its host's open state, so the host wraps each action rather + * than the menu closing itself. The item's own action runs first, then `close`. + * + * @param items The menu items to wrap + * @param close Closes the host's menu; runs after the picked item's action + * @returns New items with the same fields and wrapped actions; the input items are not modified + */ +export function wrapMarkerMenuItemsWithClose( + items: MarkerMenuItem[], + close: () => void, +): MarkerMenuItem[] { + return items.map((item) => ({ + ...item, + action: () => { + item.action(); + close(); + }, + })); +} diff --git a/extensions/src/platform-scripture-editor/src/paragraph-marker-selection-contrast.test.ts b/extensions/src/platform-scripture-editor/src/paragraph-marker-selection-contrast.test.ts new file mode 100644 index 00000000000..1599618bc58 --- /dev/null +++ b/extensions/src/platform-scripture-editor/src/paragraph-marker-selection-contrast.test.ts @@ -0,0 +1,95 @@ +// @vitest-environment node +import { readFileSync } from 'fs'; +import path from 'path'; +import chroma from 'chroma-js'; +import { describe, expect, it } from 'vitest'; + +// WCAG 2.2 non-text contrast minimum (SC 1.4.11). The selected-row bar is a UI indicator, not text. +const MIN_NON_TEXT_CONTRAST = 3; +// WCAG 2.2 text contrast minimum (SC 1.4.3). The selected marker glyph is text. +const MIN_TEXT_CONTRAST = 4.5; + +type ThemeFile = Record }>>; + +// The built-in themes as the app loads them (generated from platform-bible-react's index.css by +// its build-themes script). Read from disk so this extension test needs no import from that package. +const themesPath = path.resolve(__dirname, '../../../../src/shared/data/themes.data.json'); +// JSON.parse returns `any`, which assigns to the known theme-file shape without a type assertion +const themeFile: ThemeFile = JSON.parse(readFileSync(themesPath, 'utf-8')); + +/** + * Every theme with real variables. The file also carries `user-*` placeholder families with empty + * `cssVariables`; filtering on that rather than on names means a fifth real theme is swept + * automatically. + */ +const realThemes = Object.entries(themeFile).flatMap(([familyId, family]) => + Object.entries(family) + .filter(([, definition]) => Object.keys(definition.cssVariables).length > 0) + .map(([themeType, definition]) => ({ + name: familyId ? `${familyId}-${themeType}` : themeType, + cssVariables: definition.cssVariables, + })), +); + +const scss = readFileSync(path.resolve(__dirname, '_usj-nodes.scss'), 'utf-8').replace( + /\/\*[\s\S]*?\*\//g, + '', +); + +/** The declarations of the rule with exactly this selector, or undefined if there is none. */ +const block = (selector: string) => + scss.match( + new RegExp(`${selector.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\s*\\{([^}]*)\\}`), + )?.[1]; + +// `--foreground` is the bar token because it is the one that clears the minimum everywhere: +// `--primary` and `--ring`, the obvious alternatives, each fall short in at least one built-in theme. +describe('selected paragraph marker bar contrast', () => { + it('found real themes to check', () => { + // Guards the sweep: a filter that excluded every theme would pass every check by running none. + expect(realThemes.length).toBeGreaterThanOrEqual(4); + }); + + it('ties the sweep to the token the stylesheet actually paints the bar with', () => { + // The bar is the second (lower) box-shadow layer; the fill `--accent` layer covers all of it but + // the leading 4px. A change to another token there fails here instead of passing silently. + const bar = /box-shadow:[^;]*var\(--accent\)\s*,[^;]*var\(--foreground\)\s*;/; + expect(block('.psc-gutter-markers .psc-para-marker-selected')).toMatch(bar); + expect(block(".psc-gutter-markers[dir='rtl'] .psc-para-marker-selected")).toMatch(bar); + }); + + // The bar sits over the fill's neighbour: the editor surface (`--background`) at its outer edge + // and the row fill (`--accent`) at its inner edge. One theme passing proves nothing about the others. + realThemes.forEach(({ name, cssVariables }) => { + (['background', 'accent'] as const).forEach((surface) => { + it(`clears ${MIN_NON_TEXT_CONTRAST}:1 against --${surface} in ${name}`, () => { + const contrast = chroma.contrast( + chroma(cssVariables.foreground), + chroma(cssVariables[surface]), + ); + expect(contrast).toBeGreaterThanOrEqual(MIN_NON_TEXT_CONTRAST); + }); + }); + }); +}); + +describe('selected paragraph marker glyph contrast', () => { + it('ties the sweep to the token the stylesheet actually paints the glyph with', () => { + expect( + block( + '.psc-gutter-markers .psc-para-marker-selected > .marker:not(.verse):not(.chapter):first-child', + ), + ).toMatch(/(?:^|[\s;])color:\s*var\(--accent-foreground\)\s*;/); + }); + + // The glyph sits on the row fill (`--accent`). + realThemes.forEach(({ name, cssVariables }) => { + it(`clears ${MIN_TEXT_CONTRAST}:1 against --accent in ${name}`, () => { + const contrast = chroma.contrast( + chroma(cssVariables['accent-foreground']), + chroma(cssVariables.accent), + ); + expect(contrast).toBeGreaterThanOrEqual(MIN_TEXT_CONTRAST); + }); + }); +}); diff --git a/extensions/src/platform-scripture-editor/src/paragraph-style-trigger.component.stories.tsx b/extensions/src/platform-scripture-editor/src/paragraph-style-trigger.component.stories.tsx index 1f5520bef3d..a421b7610f7 100644 --- a/extensions/src/platform-scripture-editor/src/paragraph-style-trigger.component.stories.tsx +++ b/extensions/src/platform-scripture-editor/src/paragraph-style-trigger.component.stories.tsx @@ -1,11 +1,12 @@ import type { Meta, StoryObj } from '@storybook/react-webpack5'; import { + Button, MARKER_MENU_STRING_KEYS, SHRINK_STEP, ShrinkStepOverride, type MarkerMenuItem, } from 'platform-bible-react'; -import type { ComponentProps, ReactNode } from 'react'; +import { useRef, useState, type ComponentProps, type ReactNode } from 'react'; import { getLocalizedStrings } from '../../../../.storybook/localization.utils'; import { ParagraphStyleTrigger, @@ -25,7 +26,8 @@ import { * `ShrinkStepOverride` so every step is reachable at any browser width. * * **Try it**: click the trigger to open the marker menu, and hover the disabled story to see why - * the paragraph style cannot be changed. + * the paragraph style cannot be changed. The editor-request story opens the menu the way Enter or + * Alt+Down on a selected paragraph marker does, without a click on the trigger. */ const meta: Meta = { title: 'Bundled Extensions/platform-scripture-editor/ParagraphStyleTrigger', @@ -83,6 +85,8 @@ type TriggerArgs = ComponentProps; /** Renders the trigger with the story's args, the shared menu items, and the shared strings. */ function Trigger({ blockMarker, isStructureProtected = false, styleName }: Partial) { + // The web view owns this state in the app; each story instance keeps its own so clicking opens it. + const [isMenuOpen, setIsMenuOpen] = useState(false); return ( {}} /> ); } +/** + * Renders the trigger beside a stand-in for the editor: a button that opens the menu through the + * controlled `isMenuOpen`, as the web view does when the editor's `onParaMarkerMenuRequest` fires, + * and a text box that takes focus back whenever the menu closes normally. + */ +function EditorRequestTrigger({ + blockMarker, + isStructureProtected = false, + styleName, +}: Partial) { + const [isMenuOpen, setIsMenuOpen] = useState(false); + // The ref needs to start out with null for it to work as an element ref + // eslint-disable-next-line no-null/no-null + const editorRef = useRef(null); + return ( +
+ editorRef.current?.focus()} + /> +