diff --git a/.claude/launch.json b/.claude/launch.json new file mode 100644 index 0000000..55f44a8 --- /dev/null +++ b/.claude/launch.json @@ -0,0 +1,11 @@ +{ + "version": "0.0.1", + "configurations": [ + { + "name": "dev", + "runtimeExecutable": "npm", + "runtimeArgs": ["run", "dev", "--", "--port", "5273", "--strictPort"], + "port": 5273 + } + ] +} diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index 6d07f5d..a316870 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -29,14 +29,29 @@ build, or a "this looks wrong" after actually trying the app, leaves only throwa (`~/.cargo/bin/cargo --version` ≥ 1.88 — the Homebrew rust 1.87 is too old; see the `rust-toolchain-tauri` memory). The build script prepends `~/.cargo/bin` to PATH itself, so a Homebrew cargo sitting first on PATH no longer breaks the build. -3. **Signing credentials** in the environment (the build script reads these; never print their - values): the Apple set — `APPLE_SIGNING_IDENTITY`, `APPLE_API_KEY` (or `APPLE_API_KEY_ID`, which - the build script aliases to it), `APPLE_API_ISSUER`, `APPLE_API_KEY_PATH` (the `.p8` file is - readable) — **and** the updater key - `TAURI_SIGNING_PRIVATE_KEY` (path to / content of the passwordless - `~/Documents/Apple Connect Keys/gravity-notes-updater.key`; - `TAURI_SIGNING_PRIVATE_KEY_PASSWORD` may be empty). Check each is set — without the updater key - the build can't sign the auto-update bundle and fails. +3. **Signing credentials** — these live in **`~/.localrc`**, NOT in the agent's default + environment. The Bash tool starts a **fresh shell on every call** and does not persist env vars + between calls, so you must `source ~/.localrc` **in the same command** as anything that reads the + credentials — both this check **and** the build in Step 3 (a bare `source` in an earlier call is + gone by the next). The vars (the build script reads these; **never print their values**): the + Apple set — `APPLE_SIGNING_IDENTITY`, `APPLE_API_KEY` (or `APPLE_API_KEY_ID`, which the build + script aliases to it), `APPLE_API_ISSUER`, `APPLE_API_KEY_PATH` (the `.p8` file is readable) — + **and** the updater key `TAURI_SIGNING_PRIVATE_KEY` (path to / content of the passwordless + `~/Documents/Apple Connect Keys/gravity-notes-updater.key`; `TAURI_SIGNING_PRIVATE_KEY_PASSWORD` + may be empty). Verify each is set (names only, never values) — without the updater key the build + can't sign the auto-update bundle and fails: + + ```bash + # The Bash tool runs zsh, so this stays POSIX (no `${!v}` indirect expansion — that's a bash-ism). + test -f ~/.localrc || echo "MISSING ~/.localrc — signing creds live here" + source ~/.localrc + [ -n "$APPLE_SIGNING_IDENTITY" ] || echo "MISSING: APPLE_SIGNING_IDENTITY" + [ -n "$APPLE_API_ISSUER" ] || echo "MISSING: APPLE_API_ISSUER" + [ -n "$TAURI_SIGNING_PRIVATE_KEY" ] || echo "MISSING: TAURI_SIGNING_PRIVATE_KEY" + [ -n "$APPLE_API_KEY" ] || [ -n "$APPLE_API_KEY_ID" ] || echo "MISSING: APPLE_API_KEY" + [ -r "$APPLE_API_KEY_PATH" ] || echo "MISSING/unreadable: APPLE_API_KEY_PATH" + ``` + 4. **GitHub CLI.** `gh auth status` is logged in. 5. **Green tree.** Run `npm run typecheck`, `npm test`, and `npm run lint`. Do not release a red tree. @@ -83,8 +98,11 @@ lockstep (leave them **uncommitted** for now). Capture `$NEW` — every later st ## Step 3 — Build, sign, notarize (the slow step) +**Source the credentials in the same command as the build** — the Bash tool's shell doesn't carry +env vars over from the Step 0 check (fresh shell per call), so the build needs its own `source`: + ```bash -./scripts/build-mac-release.sh +source ~/.localrc && ./scripts/build-mac-release.sh ``` Tauri builds + signs the `.app` (hardened runtime, Developer ID) and notarizes+staples diff --git a/README.md b/README.md index 5baaf16..ef7a264 100644 --- a/README.md +++ b/README.md @@ -85,14 +85,12 @@ in-browser storage. The desktop app reads the folder natively, with no re-prompt - Restore all workspace windows on relaunch (today only the last-active one comes back) - Resizable left panel -- Icon picker polish: `aria-activedescendant` can point at a virtualized-out option; the title picker can still set an icon in preview mode; no search debounce - Preserve cmd+z between notes - Cmd+z for undoing deleting of notes and moves between folders? - Easter egg in top bar (to the right of the search bar) ? - Notion-like page title backgrounds -- Better looking checklists - Auto-empty the Trash and clean unused attachments (after 30 days?) - Metadata storage approach review diff --git a/docs/proposals/preserve-undo-across-notes.md b/docs/proposals/preserve-undo-across-notes.md new file mode 100644 index 0000000..4014426 --- /dev/null +++ b/docs/proposals/preserve-undo-across-notes.md @@ -0,0 +1,113 @@ +# Proposal: Preserve undo/redo across note switches + +Status: **proposed** (not yet implemented) · Backlog item: "Preserve cmd+z between notes" + +## Context + +Today, switching notes **hard-resets** both undo histories (ProseMirror + CodeMirror) so `⌘Z` +can't walk into the previous note's content. The cost: edit note A, switch to B, come back to A — +`⌘Z` does nothing, A's edit history is gone. This proposal makes per-note undo/redo **survive** +switching away and back, within a session. + +The editor (`@gravity-ui/markdown-editor`) is a single reused instance; content is swapped in place +per note. There is **no** public API to serialize/restore history (the `history` plugin instance is +private, so `EditorState.toJSON`/`fromJSON` with plugin fields is a dead end). The workable approach: +**hold each note's live `EditorState` object in memory and restore it wholesale** — history travels +with the object. This is exactly what the current reset already does, just with a _fresh_ state: +`view.updateState(EditorState.create({doc, plugins}))` (`EditorPane.tsx`, `resetHistory`). We swap the +fresh state for the saved one. + +All changes are contained to `src/components/EditorPane.tsx` (the swap logic), with doc updates. + +## Approach + +Keep two per-note maps of live editor states (one per mode), save the **active** mode's state when +switching away, and restore it when returning — but only if it's still valid. + +### Why active-mode-only + +The hidden mode's buffer goes stale between mode toggles (e.g. editing in WYSIWYG leaves the +CodeMirror buffer showing old content until a toggle regenerates it). So its saved state wouldn't +match `editor.getValue()`. Saving/restoring **only the currently-active mode** (`editor.currentMode`) +keeps the stored content in lockstep with the saved doc. The inactive mode keeps today's fresh-reset +behavior (which also re-syncs its hidden buffer to the new content — load-bearing, see the +`resetMarkupHistory` comment in `EditorPane.tsx`). + +### Validity guard + +A saved state is only restored when its stored `content === note.content` (the incoming, on-disk +content). Pending edits are flushed before `open()`, so on a normal switch-back these match. If the +note changed on disk while away (external edit / conflict reload), they won't — fall back to a fresh +reset so we never restore stale history over new content. + +### Memory footprint + +A saved `EditorState` is dominated by its immutable document tree (roughly 2–5× the note's text size +in JS object overhead) plus the `history` plugin's stack (`history({depth: 100})` — bounded; small +for normal typing). Typical note → ~20–100 KB; a large 100 KB note → a few hundred KB. Both maps are +**LRU-capped at ~25 notes**, so the worst case (25 huge, heavily-edited notes) is the only place this +grows, and the cap bounds it. The cap value is the effective lever if the footprint needs tuning. + +## Changes — `src/components/EditorPane.tsx` + +1. **Two LRU-capped ref maps** beside the existing `viewStateByIdRef`: + - `pmHistByIdRef: Map` + - `cmHistByIdRef: Map` + - Cap ~25 entries each (delete-then-set for LRU ordering; evict oldest past the cap). + +2. **Extend `saveViewState(id)`** to also capture the active mode's live state with the current + content: + - `wysiwyg` → `pmHistByIdRef.set(id, {state: wikiViewRef.current.state, content: editor.getValue()})` + - `markup` → `cmHistByIdRef.set(id, {state: markupEditorOf(editor).cm.state, content: editor.getValue()})` + +3. **Turn the two resets into restore-or-reset** (return `preserved: boolean`): + - `resetHistory(id)`: if `currentMode === 'wysiwyg'` and a saved PM entry's + `content === note.content` → `view.updateState(saved.state)` and return `true`. Else the current + fresh `EditorState.create({doc, plugins})` path, return `false`. + - `resetMarkupHistory(id)`: symmetric with `markup.cm.setState(saved.state)` vs the current + `freshMarkupState(template, note.content)`. + +4. **Swap effect**: pass `note.id` into both resets; capture `preserved` from `resetHistory`. A + restored PM state already carries its own selection, so **skip `restoreSelection`** when preserved + (calling it would dispatch a redundant `setSelection` — which would pollute the just-restored + history). Keep it for the fresh path. + +5. **Re-key on rename/move**: carry `pmHistByIdRef` / `cmHistByIdRef` entries from the old id to the + new one, alongside the existing `viewStateByIdRef` re-key. + +Reused as-is: `markupEditorOf` / `freshMarkupState` (`editor/markupHistory.ts`), `editor.currentMode` +(already used in `EditorPane.tsx`), the `swappingRef` bracket (updateState/setState run inside it, so +the change handler still treats them as a load echo, not a user edit). + +## Docs + +- Update the class comment in `EditorPane.tsx` (the "history HARD-RESET" note) and the + `resetHistory`/`resetMarkupHistory` doc comments to describe restore-or-reset. +- Update `CLAUDE.md` (the EditorPane line stating a note switch "hard-resets BOTH undo histories"). + +## Verification + +**Live (primary), via the preview dev server:** + +1. WYSIWYG: type in note A, switch to B, type in B, switch back to A → `⌘Z` undoes A's edits (not + B's, not nothing); `⌘⇧Z` redoes. Switch to B → `⌘Z` undoes B's. Confirm undo stops at each note's + loaded state and never crosses into the other note's text. +2. Markup mode (`⌘⇧;`): repeat — history preserved per note there too. +3. Fresh open of a never-visited note → `⌘Z` is a no-op (nothing to undo). Rename a note mid-session, + switch away and back → history still restores under the new name. + +**Tests:** add coverage where deterministic in jsdom — extend the editor-history tests +(`editor/markupHistory.test.ts` is the closest existing harness) and/or add an EditorPane/Workspace +integration test that types in A, switches to B and back, dispatches undo, and asserts A's content +reverts. Full suite (`npm test`), `npm run typecheck`, `npm run lint`, `npm run format:check`. + +## Risks & mitigations + +- **Restoring a bad/stale state** → guarded by content-match + active-mode checks; any miss falls + back to today's fresh reset (the existing, safe behavior). +- **Memory growth** → both maps LRU-capped (~25 notes). +- **Cross-mode / conflict-reload edge cases** → the content-match guard makes these fall back to + fresh reset, matching current behavior. +- This is the app's most delicate code (the swap logic); the fresh-reset paths are left intact as the + fallback, so the blast radius of a preserve-path bug is "history resets like before," not + corruption. diff --git a/docs/shortcuts.md b/docs/shortcuts.md index e2d043b..3dc9d28 100644 --- a/docs/shortcuts.md +++ b/docs/shortcuts.md @@ -16,21 +16,21 @@ The search box is the heart of the app (nvALT-style **search or create**): ## Navigation -| Keys | Action | -| ------------------------ | ------------------------------------------------------------------------------------- | -| `↑` / `↓` (or `k` / `j`) | Preview the previous / next note | -| `⌘J` / `⌘K` | Preview next / previous note (works while editing) | -| `⌘[` / `⌘]` | Go back / forward through visited notes (browser-style history) | -| `Enter` | Edit the selected note (in the title → jump to the body) | -| `⌘Enter` / `⌘-click` | Open the selected note in its own window _(desktop)_ | -| `⌘0` | Show this workspace's main window _(desktop; also Window ▸ Main Window)_ | -| `Esc` | Editor → list → search (then close / clear) | -| `Esc` `Esc` | Focus the search box | -| `⌘L` | Jump to the search box (desktop app; browsers reserve it for the address bar) | -| `⌘\` | Toggle the sidebar | -| `⌘⇧\` | Toggle the folder rail | -| `⌘'` | Peek the collapsed sidebar / focus the list (again to close) | -| `⌃R` | Switch workspace — the recent-folders dialog (`↵` open, `⌘↵` new window, `⌘⌫` remove) | +| Keys | Action | +| ----------------------------------- | ------------------------------------------------------------------------------------- | +| `↑` / `↓` (or `k` / `j`) | Preview the previous / next note | +| `⌘J` / `⌘K` | Preview next / previous note (works while editing) | +| `⌘[` / `⌘]` | Go back / forward through visited notes (browser-style history) | +| `Enter` | Edit the selected note (in the title → jump to the body) | +| `⌘Enter` / `⌘-click` / double-click | Open the selected note in its own window _(desktop)_ | +| `⌘0` | Show this workspace's main window _(desktop; also Window ▸ Main Window)_ | +| `Esc` | Editor → list → search (then close / clear) | +| `Esc` `Esc` | Focus the search box | +| `⌘L` | Jump to the search box (desktop app; browsers reserve it for the address bar) | +| `⌘\` | Toggle the sidebar | +| `⌘⇧\` | Toggle the folder rail | +| `⌘'` | Peek the collapsed sidebar / focus the list (again to close) | +| `⌃R` | Switch workspace — the recent-folders dialog (`↵` open, `⌘↵` new window, `⌘⌫` remove) | ## Editing diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 5fd8f65..2cea241 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -806,7 +806,7 @@ fn watch_rel_path(canon_root: &Path, path: &Path) -> Option { } match fs::symlink_metadata(path) { Ok(meta) if meta.is_dir() => Some(segments.join("/")), - Ok(_) => None, // an existing non-md file — not note-relevant + Ok(_) => None, // an existing non-md file — not note-relevant Err(_) => Some(segments.join("/")), // gone/unreadable — unclassifiable, include } } @@ -843,7 +843,10 @@ fn remove_window_subscriptions(watchers: &Watchers, label: &str) -> Vec Vec::new(), + Ok(events) + if events + .iter() + .any(|event| event.path.as_path() == canon_root) => + { + Vec::new() + } Ok(events) => { // BTreeSet: dedup (one save fires several events per file) + stable order. let set: std::collections::BTreeSet = events @@ -2254,7 +2263,10 @@ mod tests { let rel = |p: &Path| watch_rel_path(&root, p); // Notes pass as POSIX rel-paths — existing or already deleted (a deleted path can't be // classified, and a missed deletion would be a bug). - assert_eq!(rel(&root.join("Work/Sub/Deep.md")), Some("Work/Sub/Deep.md".into())); + assert_eq!( + rel(&root.join("Work/Sub/Deep.md")), + Some("Work/Sub/Deep.md".into()) + ); assert_eq!(rel(&root.join("Note.md")), Some("Note.md".into())); // A dot-NAMED note passes: the note walks skip dot-DIRS only and do list `.hidden.md`, // so the watcher must report its changes too (listed-but-never-refreshed otherwise). diff --git a/src/components/EditorPane.css b/src/components/EditorPane.css index 19d391a..96e3dce 100644 --- a/src/components/EditorPane.css +++ b/src/components/EditorPane.css @@ -193,12 +193,58 @@ content: ''; } -/* Breathing room between a checklist checkbox and its label, plus vertical centering against - the first text line. */ +/* Tighten the gap between checklist rows — the editor gives each `.g-md-checkbox` a 15px bottom + margin (paragraph rhythm), which reads loose for a checklist; pull it in for a denser, more + Notion-like list. */ +.editor-pane .g-md-editor .g-md-checkbox { + margin-bottom: 6px; +} + +/* Notion-style checkbox: a custom-drawn rounded square (native chrome stripped via + appearance:none) that fills with the app accent and shows a white check when ticked. The check is + a background SVG, not a pseudo-element — is a replaced element and WebKit won't render + ::before/::after on it. Both the editor (here) and the read-only preview (NotePreview.css) share + the look; keep the two in sync. */ .editor-pane .g-md-editor .g-md-checkbox__input[type='checkbox'] { - margin-right: 10px; - margin-top: 0; - vertical-align: middle; + appearance: none; + -webkit-appearance: none; + box-sizing: border-box; + flex: none; + width: 17px; + height: 17px; + /* Vertical alignment: the plugin pins the box to the row's top (align-self:flex-start) and shifts + it 3px down with a positioned `top` — neutralize that, then center the box within ONE line via + margin-top. `1lh` (= the inherited text line-height) minus the box height, halved, centres the + 17px box on the first text line at any font size (and keeps it on the FIRST line when the item + wraps, since flex-start still pins to the top). */ + position: relative; + top: 0; + align-self: flex-start; + line-height: inherit; + margin: calc((1lh - 17px) / 2) 7px 0 0; + border: 1.5px solid var(--g-color-line-generic-hover); + border-radius: 5px; + background-color: transparent; + background-repeat: no-repeat; + background-position: center; + background-size: 12px 12px; + cursor: pointer; + transition: + background-color 0.12s ease, + border-color 0.12s ease; +} + +/* Hover on an unticked box: a faint accent-tinted fill + accent border, matching Notion's affordance. */ +.editor-pane .g-md-editor .g-md-checkbox__input[type='checkbox']:hover:not(:checked) { + border-color: var(--gn-orange); + background-color: rgba(var(--gn-accent-rgb), 0.12); +} + +/* Ticked: accent fill + a white check glyph. */ +.editor-pane .g-md-editor .g-md-checkbox__input[type='checkbox']:checked { + border-color: var(--gn-orange); + background-color: var(--gn-orange); + background-image: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 16 16'%3E%3Cpath fill='none' stroke='white' stroke-width='2.2' stroke-linecap='round' stroke-linejoin='round' d='M3.5 8.5l3 3 6-7'/%3E%3C/svg%3E"); } /* Checklist rows should indent like the dashed (bulleted) lists — the checkbox box adds extra diff --git a/src/components/IconPicker.test.tsx b/src/components/IconPicker.test.tsx new file mode 100644 index 0000000..17e0fd9 --- /dev/null +++ b/src/components/IconPicker.test.tsx @@ -0,0 +1,88 @@ +import {act, fireEvent, screen, waitFor} from '@testing-library/react'; +import {afterEach, beforeAll, describe, expect, it, vi} from 'vitest'; + +import {loadEmojis, loadIconCatalog} from '../icons'; +import {renderWithProviders} from '../test/render'; + +import {IconPicker} from './IconPicker'; + +// The picker lazy-loads its emoji + icon catalogs on first open; preload them once (both memoized at +// module scope) so options render synchronously — same trick the NoteList icon-picker tests use. +beforeAll(async () => { + await Promise.all([loadEmojis(), loadIconCatalog()]); +}); + +afterEach(() => { + vi.useRealTimers(); +}); + +/** Open a fresh self-contained picker and return the combobox (search box) once the grid is live. */ +async function openPicker(props: Partial[0]> = {}) { + const result = renderWithProviders(); + fireEvent.click(screen.getByRole('button', {name: 'Set note icon'})); + const combobox = await screen.findByRole('combobox'); + await waitFor(() => expect(screen.getAllByRole('option').length).toBeGreaterThan(0)); + return {...result, combobox}; +} + +describe('IconPicker', () => { + it('keeps the keyboard-highlighted option mounted, so aria-activedescendant never dangles', async () => { + const {combobox} = await openPicker(); + // Step into the grid, then rove far past the initial virtual window (8 cols/row → row 24). + for (let i = 0; i < 25; i++) fireEvent.keyDown(combobox, {key: 'ArrowDown'}); + + const activeId = combobox.getAttribute('aria-activedescendant'); + expect(activeId).toBeTruthy(); + // Without the rangeExtractor forcing the active row to render, this element would be + // virtualized out and getElementById would return null — a dangling aria reference. + const target = document.getElementById(activeId ?? ''); + expect(target).not.toBeNull(); + expect(target?.getAttribute('role')).toBe('option'); + expect(target?.className).toMatch(/icon-picker__item_active/); + }); + + it('does not crash when the highlight is roved past a then-shrunken result set', async () => { + const {combobox} = await openPicker(); + // Rove into the grid so the highlight sits on row 3 (activeIndex 24). + for (let i = 0; i < 4; i++) fireEvent.keyDown(combobox, {key: 'ArrowDown'}); + // Narrow to a tiny (non-empty) set — the roved row now lies PAST the shrunken list. With the + // unguarded rangeExtractor this pushed an out-of-range index into the virtualizer and crashed + // the render (`measurements[i]` undefined → `virtualRow.key` deref). + fireEvent.change(combobox, {target: {value: 'unicorn'}}); + await waitFor(() => { + const opts = screen.getAllByRole('option'); + expect(opts.length).toBeGreaterThan(0); + // One row's worth or fewer — proves the set shrank below the roved row (row 3). + expect(opts.length).toBeLessThan(9); + }); + }); + + it('closes an open popup when it becomes disabled (entering read-only preview mode)', async () => { + const {rerender} = await openPicker(); + expect(screen.getByRole('listbox', {name: 'Pick an icon'})).toBeInTheDocument(); + + // Toggling into preview mode disables the picker while its popup is still up. + rerender(); + await waitFor(() => + expect(screen.queryByRole('listbox', {name: 'Pick an icon'})).toBeNull(), + ); + }); + + it('debounces the search query before filtering the grid', async () => { + const {combobox} = await openPicker(); + const before = screen.getAllByRole('option').length; + expect(before).toBeGreaterThan(0); + + vi.useFakeTimers(); + // A query nothing matches — once applied, the grid empties. + fireEvent.change(combobox, {target: {value: 'zzznomatchxyz'}}); + // Debounce window (100 ms) hasn't elapsed: the grid still shows the pre-query options. + expect(screen.getAllByRole('option').length).toBe(before); + + act(() => { + vi.advanceTimersByTime(150); + }); + // Now the filter has run against the settled query → no matches left. + expect(screen.queryAllByRole('option')).toHaveLength(0); + }); +}); diff --git a/src/components/IconPicker.tsx b/src/components/IconPicker.tsx index 4753fec..e592dff 100644 --- a/src/components/IconPicker.tsx +++ b/src/components/IconPicker.tsx @@ -12,8 +12,9 @@ import type {KeyboardEvent as ReactKeyboardEvent, MouseEvent as ReactMouseEvent} import {Button, Icon, Popup, SegmentedRadioGroup, TextInput} from '@gravity-ui/uikit'; import type {ButtonProps} from '@gravity-ui/uikit'; -import {useVirtualizer} from '@tanstack/react-virtual'; +import {defaultRangeExtractor, useVirtualizer} from '@tanstack/react-virtual'; +import {useDebouncedValue} from '../hooks/useDebouncedValue'; import { type EmojiItem, type IconItem, @@ -131,6 +132,10 @@ export const IconPickerPopup = memo(function IconPickerPopup({ // Open is the anchor's presence — one prop, so no caller can render an anchorless open popup. const open = anchorElement !== null; const [query, setQuery] = useState(''); + // Debounce the query that DRIVES filtering — the catalog is thousands of icons + emoji, so + // re-filtering and re-virtualizing on every keystroke is wasteful. The input stays bound to the + // live `query` for responsive typing; only the grid reads the settled value. + const debouncedQuery = useDebouncedValue(query, 100); const [type, setType] = useState('all'); const [emojis, setEmojis] = useState(null); const [icons, setIcons] = useState(() => getIconCatalog()?.all ?? null); @@ -168,7 +173,7 @@ export const IconPickerPopup = memo(function IconPickerPopup({ const out: Entry[] = []; // In the "All" view, emoji come first, then the Gravity symbols. if (type !== 'icons' && emojis) { - for (const emoji of filterEmojis(query, emojis)) { + for (const emoji of filterEmojis(debouncedQuery, emojis)) { out.push({ kind: 'emoji', key: `emoji:${emoji.char}`, @@ -179,7 +184,7 @@ export const IconPickerPopup = memo(function IconPickerPopup({ } } if (type !== 'emoji' && icons) { - for (const icon of filterIcons(query, icons)) { + for (const icon of filterIcons(debouncedQuery, icons)) { out.push({ kind: 'icon', key: icon.name, @@ -189,7 +194,7 @@ export const IconPickerPopup = memo(function IconPickerPopup({ } } return out; - }, [open, query, type, emojis, icons]); + }, [open, debouncedQuery, type, emojis, icons]); // Chunk the flat entry list into fixed 8-wide rows so we can virtualize by row: the catalog runs // to thousands of icons/emoji and mounting them all was what made the popup lag. @@ -199,12 +204,29 @@ export const IconPickerPopup = memo(function IconPickerPopup({ return out; }, [entries]); + // The grid row holding the keyboard highlight. Forced to stay mounted below so the search box's + // `aria-activedescendant` always points at a real element — a highlighted option scrolled out of + // the virtual window would otherwise leave the attribute dangling at a torn-down id. + const activeRow = activeIndex >= 0 ? Math.floor(activeIndex / COLUMNS) : -1; const scrollRef = useRef(null); const rowVirtualizer = useVirtualizer({ count: rows.length, getScrollElement: () => scrollRef.current, estimateSize: () => ROW_HEIGHT, overscan: 4, + // A fresh closure per render keeps `activeRow` current (matches NoteList's row list). The + // window is a few sorted indexes, so the extra push + sort is free. + rangeExtractor: (range) => { + const indexes = defaultRangeExtractor(range); + // Guard `< rows.length`: a query can shrink the grid a render before the roving + // highlight resets (that reset is a post-commit effect), leaving `activeRow` past the + // new list — pushing it would hand the virtualizer an out-of-range index and crash the + // render (`measurements[i]` is undefined → `virtualRow.key` deref). + if (activeRow >= 0 && activeRow < rows.length && !indexes.includes(activeRow)) { + indexes.push(activeRow); + } + return indexes.sort((a, b) => a - b); + }, }); // The Popup unmounts its content on close, so on reopen the virtualizer is left measuring a stale @@ -399,6 +421,12 @@ export function IconPicker({value, onChange, size = 'm', disabled, className}: I const onOpenChange = useCallback((next: boolean) => { if (!next) setAnchor(null); }, []); + // Close the popup if the picker becomes disabled while it's open — e.g. toggling the note into + // read-only preview mode (⌘⇧P) with the popup up. The disabled BUTTON only blocks opening; an + // already-open popup would otherwise stay pickable and commit a change preview should forbid. + useEffect(() => { + if (disabled) setAnchor(null); + }, [disabled]); return ( <> { fireEvent.click(screen.getByRole('option', {name: /Beta/}), {metaKey: true}); expect(props.onBrowse).toHaveBeenCalledWith('Beta.md'); }); + + it('double-click on a row opens it in a new window', () => { + const onOpenInNewWindow = vi.fn(); + setup({onOpenInNewWindow}); + fireEvent.doubleClick(screen.getByRole('option', {name: /Beta/})); + expect(onOpenInNewWindow).toHaveBeenCalledWith('Beta.md'); + }); + + it('double-click stays inert without the callback (web build)', () => { + setup(); + // No throw, no crash — just nothing to open a window with. + fireEvent.doubleClick(screen.getByRole('option', {name: /Beta/})); + }); }); describe('NoteList — folder scope chip (rail closed)', () => { @@ -772,5 +785,5 @@ describe('NoteList — note icons (one shared picker)', () => { await user.click(within(alpha).getByRole('button', {name: /note icon/i})); await screen.findByRole('listbox', {name: 'Pick an icon'}); expect(screen.getByRole('radio', {name: 'All'})).toBeChecked(); - }, 15_000); + }); }); diff --git a/src/components/NoteList.tsx b/src/components/NoteList.tsx index 631449c..c67a1dd 100644 --- a/src/components/NoteList.tsx +++ b/src/components/NoteList.tsx @@ -194,6 +194,7 @@ interface NoteRowProps { onOpenIconPicker: (id: string, anchor: HTMLElement) => void; showIcons: boolean; onClickRow: (id: string, event: ReactMouseEvent) => void; + onDoubleClickRow: (id: string, event: ReactMouseEvent) => void; onContextMenuRow: (note: NoteMeta, x: number, y: number) => void; onKeyDownRow: (event: ReactKeyboardEvent, id: string) => void; onOpenMenu: (note: NoteMeta, anchor: HTMLElement) => void; @@ -224,6 +225,7 @@ const NoteRow = memo(function NoteRow({ onOpenIconPicker, showIcons, onClickRow, + onDoubleClickRow, onContextMenuRow, onKeyDownRow, onOpenMenu, @@ -247,6 +249,7 @@ const NoteRow = memo(function NoteRow({ e.dataTransfer.setData('text/plain', note.id); }} onClick={(e) => onClickRow(note.id, e)} + onDoubleClick={(e) => onDoubleClickRow(note.id, e)} onMouseDown={(e) => { // Right-click and ⌘-click act on a row WITHOUT selecting it — block the mousedown // default so the focusable row doesn't grab DOM focus either (the context menu / @@ -628,6 +631,18 @@ export const NoteList = forwardRef(function NoteL [browseRow], ); + // Double-click (desktop): open the note in its own window — Apple-Notes-style, matching ⌘↵ / + // ⌘-click / the ⋯ "Open in New Window" item. The preceding single click already browsed+selected + // it in this window, which is fine. No-op on web (no callback) — the browser's word-select on + // double-click is harmless there, so nothing to prevent. + const onDoubleClickRow = useCallback((id: string, event: ReactMouseEvent) => { + const {editingId: editing, onOpenInNewWindow: openInNew} = live.current; + if (editing === id || !openInNew) return; + // Suppress the accompanying word-selection so the row doesn't flash a highlighted title. + event.preventDefault(); + openInNew(id); + }, []); + // Deliberately NO browse here: right-click acts on the clicked note via the menu's own // `note` payload, without moving the selection/preview off whatever is open (same rule as // ⌘-click / ⌘↵ opening a new window). @@ -915,6 +930,7 @@ export const NoteList = forwardRef(function NoteL onOpenIconPicker={onOpenIconPicker} showIcons={showIcons} onClickRow={onClickRow} + onDoubleClickRow={onDoubleClickRow} onContextMenuRow={onContextMenuRow} onKeyDownRow={onKeyDownRow} onOpenMenu={onOpenMenu} diff --git a/src/components/NotePreview.css b/src/components/NotePreview.css index 23e51cd..a8af2c4 100644 --- a/src/components/NotePreview.css +++ b/src/components/NotePreview.css @@ -130,3 +130,45 @@ .note-preview__body.yfm ul > li:has(input[type='checkbox'])::marker { content: ''; } + +/* Notion-style checkbox, mirroring the editor (EditorPane.css) so the read-only render matches the + WYSIWYG one — a custom rounded square that fills with the accent + a white check when ticked. The + preview's checkboxes are `disabled` (read-only), so no hover affordance. Keep in sync with the + editor rule. The `.note-preview` prefix lifts specificity above diplodoc's own + `.yfm .checkbox > input[type=checkbox]` rule (yfm.css), which otherwise ties and could win on + source order — margin/alignment/`top` are all set here so nothing leaks through from it. */ +.note-preview .note-preview__body.yfm input[type='checkbox'] { + appearance: none; + -webkit-appearance: none; + box-sizing: border-box; + flex: none; + /* Centre the box on the first text line — see EditorPane.css for the `1lh` reasoning. diplodoc + shifts the input down with a positioned `top`; neutralize it, then centre via margin-top. */ + position: relative; + top: 0; + align-self: flex-start; + line-height: inherit; + width: 17px; + height: 17px; + margin: calc((1lh - 17px) / 2) 7px 0 0; + padding: 0; + border: 1.5px solid var(--g-color-line-generic-hover); + border-radius: 5px; + background-color: transparent; + background-repeat: no-repeat; + background-position: center; + background-size: 12px 12px; + /* Read-only, but keep it looking un-disabled (full contrast, not the browser's greyed chrome). */ + opacity: 1; +} +.note-preview .note-preview__body.yfm input[type='checkbox']:checked { + border-color: var(--gn-orange); + background-color: var(--gn-orange); + background-image: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 16 16'%3E%3Cpath fill='none' stroke='white' stroke-width='2.2' stroke-linecap='round' stroke-linejoin='round' d='M3.5 8.5l3 3 6-7'/%3E%3C/svg%3E"); +} + +/* Tighten the gap between checklist rows to match the editor (its .g-md-checkbox rule) — diplodoc + gives each `.checkbox` a 15px bottom margin, looser than a checklist wants. */ +.note-preview .note-preview__body.yfm .checkbox { + margin-bottom: 6px; +} diff --git a/src/components/NotePreview.tsx b/src/components/NotePreview.tsx index 1dea99d..f12f9b6 100644 --- a/src/components/NotePreview.tsx +++ b/src/components/NotePreview.tsx @@ -8,6 +8,12 @@ import * as cutExtension from '@diplodoc/cut-extension'; // runtime; the runtime only adds the focus-into-a-collapsed-cut reveal. import '@diplodoc/cut-extension/runtime'; import transform from '@diplodoc/transform'; +// diplodoc's checkbox plugin ships inside @diplodoc/transform but isn't in its default plugin set — +// without it, GFM task lists (`- [ ] item`) leak through as literal `[ ] item` text in preview. It +// lives at a deep path (no re-export from the package root; the package has no `exports` map, so the +// subpath import resolves). `module.exports = fn`, so it's dug out with resolveCjsExport like the +// others below. +import * as checkboxExtension from '@diplodoc/transform/lib/plugins/checkbox'; import {Text} from '@gravity-ui/uikit'; import {type AttachmentUrlCache, useAttachmentCache} from '../attachments'; @@ -29,6 +35,8 @@ import '@diplodoc/cut-extension/runtime/styles.css'; // markdown-it plugin); interactivity comes from the runtime imported above. // • disableCommonAnchors — the editor shows no `#` anchor buttons beside headings; diplodoc adds // them by default, so turn them off to match. +// • checkbox — `- [ ] item` / `- [x] item` → a real (read-only) + label, +// matching the editor's WYSIWYG checkboxes. Without it the raw `[ ]`/`[x]` markup shows through. // Both packages are CJS, and bundlers expose their shape inconsistently: under Node the named export // binds directly, but Vite pre-bundles them so the export sits nested under `default` (the whole // `module.exports` object) — a bare named/default import resolves to `undefined` in one env or the @@ -46,6 +54,7 @@ function resolveCjsExport(mod: unknown, name: string): unknown { type PluginFactory = (options?: unknown) => unknown; const colorPlugin = resolveCjsExport(colorExtension, 'colorPlugin'); const cutTransform = resolveCjsExport(cutExtension, 'transform') as PluginFactory; +const checkboxPlugin = resolveCjsExport(checkboxExtension, 'checkbox'); /** * markdown-it plugin: keep linkify to EXPLICIT schemes only (https://…, mailto:…). Fuzzy matching @@ -57,12 +66,20 @@ function noFuzzyLinkify(md: {linkify: {set(opts: Record): unkno md.linkify.set({fuzzyLink: false, fuzzyEmail: false}); } +// `bundle: false` on the cut factory is load-bearing: it otherwise defaults to bundling its runtime +// assets to disk via Node `fs`, which throws in the browser (fs is externalized) and fails the whole +// transform. We ship the runtime ourselves (the side-effect import above), so no bundling. The array +// is cast to `any[]` — markdown-it plugins don't match diplodoc's ExtendedPluginWithCollect type. +const TRANSFORM_PLUGINS = [ + colorPlugin, + cutTransform({bundle: false}), + checkboxPlugin, + noFuzzyLinkify, + // eslint-disable-next-line @typescript-eslint/no-explicit-any +].filter(Boolean) as any[]; + const TRANSFORM_OPTIONS = { - // `bundle: false` is load-bearing: the cut factory otherwise defaults to bundling its runtime - // assets to disk via Node `fs`, which throws in the browser (fs is externalized) and fails the - // whole transform. We ship the runtime ourselves (the side-effect import above), so no bundling. - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- markdown-it plugins vs diplodoc's ExtendedPluginWithCollect type - plugins: [colorPlugin, cutTransform({bundle: false}), noFuzzyLinkify].filter(Boolean) as any[], + plugins: TRANSFORM_PLUGINS, disableCommonAnchors: true, // Render bare URLs as links (explicit schemes only — see noFuzzyLinkify), matching the editor. linkify: true, diff --git a/src/components/NoteTitle.tsx b/src/components/NoteTitle.tsx index 1d5c270..5d43497 100644 --- a/src/components/NoteTitle.tsx +++ b/src/components/NoteTitle.tsx @@ -148,7 +148,11 @@ export const NoteTitle = forwardRef(function No className="note-title__icon" value={icon} disabled={readOnly} - onChange={(name) => onSetIcon?.(name)} + // Belt-and-suspenders: the disabled button + close-on-disable already block this in + // preview mode, but never write an icon while read-only. + onChange={(name) => { + if (!readOnly) onSetIcon?.(name); + }} size="l" /> ) : null} diff --git a/src/hooks/useAppUpdater.test.tsx b/src/hooks/useAppUpdater.test.tsx index 2e364cf..e7a90bf 100644 --- a/src/hooks/useAppUpdater.test.tsx +++ b/src/hooks/useAppUpdater.test.tsx @@ -159,6 +159,32 @@ describe('useAppUpdater', () => { expect(result.current.error).toBeNull(); }); + it('check() passes a timeout so a stalled manifest fetch cannot hang forever', async () => { + checkMock.mockResolvedValue(null); + const {result} = renderHook(() => useAppUpdater()); + await act(async () => { + await result.current.check(); + }); + expect(checkMock).toHaveBeenCalledWith({timeout: 15_000}); + }); + + it('install() passes a download timeout so a 0-B stall cannot hang forever', async () => { + const downloadAndInstall = vi.fn(async (cb?: (e: DownloadEvent) => void) => { + cb?.({event: 'Started', data: {contentLength: 10}}); + cb?.({event: 'Finished'}); + }); + checkMock.mockResolvedValue(makeUpdate(downloadAndInstall)); + const {result} = renderHook(() => useAppUpdater()); + await act(async () => { + await result.current.check(); + }); + await act(async () => { + await result.current.install(); + }); + // Second arg to downloadAndInstall(cb, options) carries the timeout. + expect(downloadAndInstall).toHaveBeenCalledWith(expect.any(Function), {timeout: 120_000}); + }); + it('a failed install is retryable on the still-held handle', async () => { const downloadAndInstall = vi .fn() diff --git a/src/hooks/useAppUpdater.ts b/src/hooks/useAppUpdater.ts index 3bd565a..9319887 100644 --- a/src/hooks/useAppUpdater.ts +++ b/src/hooks/useAppUpdater.ts @@ -8,6 +8,18 @@ import {isTauri} from '../isTauri'; // reached via dynamic `import()` so it never enters the browser bundle (mirrors the plugin-dialog // convention). +// Neither the manifest fetch nor the binary download has a timeout by default, so a stalled +// connection (captive portal, half-open socket, a download that never leaves 0 B) hangs the flow +// forever — pinning the UI on "Checking for updates…" / "0 B" and the `busyRef` latch with it (so no +// later check or retry could run either). The plugin maps both to reqwest's whole-request timeout +// (connect → full body read), so they double as connect timeouts. +// • check: a tiny JSON — 15 s is plenty. +// • download: the update artifact is ~6 MB, so 2 min clears it on any usable link (even ~0.4 Mbps) +// while still bailing out of a true stall. NOT so tight it would abort a legitimately slow +// download — 6 MB just isn't big enough for that to be a real risk. +const CHECK_TIMEOUT_MS = 15_000; +const DOWNLOAD_TIMEOUT_MS = 120_000; + export type UpdaterStatus = | 'idle' | 'checking' @@ -92,7 +104,7 @@ export function useAppUpdater(): AppUpdater { // resource); ignore close errors — a stale handle is harmless. await updateRef.current?.close().catch(() => {}); updateRef.current = null; - const update = await runCheck(); + const update = await runCheck({timeout: CHECK_TIMEOUT_MS}); if (!update) { setInfo(null); // No Update handle means no version to read from it — ask the app shell directly so @@ -142,21 +154,24 @@ export function useAppUpdater(): AppUpdater { setProgress({downloaded: 0, total: null}); setStatus('downloading'); try { - await update.downloadAndInstall((event) => { - if (event.event === 'Started') { - setProgress({downloaded: 0, total: event.data.contentLength ?? null}); - } else if (event.event === 'Progress') { - setProgress((prev) => ({ - downloaded: (prev?.downloaded ?? 0) + event.data.chunkLength, - total: prev?.total ?? null, - })); - } else if (event.event === 'Finished') { - // Pin the bar to 100% — the last Progress chunk leaves it just short. - setProgress((prev) => - prev && prev.total !== null ? {...prev, downloaded: prev.total} : prev, - ); - } - }); + await update.downloadAndInstall( + (event) => { + if (event.event === 'Started') { + setProgress({downloaded: 0, total: event.data.contentLength ?? null}); + } else if (event.event === 'Progress') { + setProgress((prev) => ({ + downloaded: (prev?.downloaded ?? 0) + event.data.chunkLength, + total: prev?.total ?? null, + })); + } else if (event.event === 'Finished') { + // Pin the bar to 100% — the last Progress chunk leaves it just short. + setProgress((prev) => + prev && prev.total !== null ? {...prev, downloaded: prev.total} : prev, + ); + } + }, + {timeout: DOWNLOAD_TIMEOUT_MS}, + ); } catch (err) { // Download/install failed — nothing was swapped in. The found handle is kept, so the // dialog can retry install() on it. diff --git a/vite.config.ts b/vite.config.ts index 894734d..6c0c36e 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -27,10 +27,12 @@ export default defineConfig(({mode}) => ({ environment: 'jsdom', include: ['src/**/*.test.tsx'], // Rendering real Gravity components in jsdom (esp. the icon-picker popup, which - // mounts the full emoji/icon catalog) runs 2-5s per test locally and ~2.5x that - // under CI's loaded runner — past Vitest's 5s default. Give the whole DOM suite - // headroom so boundary-slow tests don't flake on CI. - testTimeout: 15_000, + // mounts the full emoji/icon catalog + drives userEvent through a virtualized + // grid) runs several seconds per test locally and more under CI's loaded runner, + // where parallel worker contention adds up — a boundary-slow test hit ~16s past + // Vitest's 5s default. Give the whole DOM suite generous headroom so these don't + // flake on CI. + testTimeout: 30_000, setupFiles: ['./src/test/setup.ts'], server: { deps: {