Skip to content

Guard keyboard shortcuts in dialogs/inputs; sync notebook emoji from meta - #49

Merged
setzor merged 1 commit into
mainfrom
vibe/shortcut-guards-emoji-sync-3d935b
Oct 27, 2026
Merged

setzor merged 1 commit into
mainfrom
vibe/shortcut-guards-emoji-sync-3d935b

Conversation

@setzor

@setzor setzor commented Oct 27, 2026

Copy link
Copy Markdown
Owner

Summary

Verification

  • pnpm test — 20 files, 93 tests passed, including new shortcuts.test.ts (popover/dialog/input/contenteditable/button-arrow cases) and notebookmeta.test.ts (emoji set/clear/preserve/unknown-id cases).
  • pnpm typecheck and pnpm build pass.

Closes #17
Closes #16

…meta

- Extract shouldSkipShortcut(): skip t/arrow shortcuts while the shelf
  popover, delete dialog, or settings overlay is open; skip INPUT,
  TEXTAREA, SELECT and contenteditable targets (attribute check too);
  leave arrow keys on buttons for focus movement (#17).
- Extract applyNotebookMeta(): onMetaChange now syncs nb.emoji (and
  handles null) so the in-memory list cannot desync from the manifest
  after renames/re-fetches (#16).

Co-authored-by: setzor <setzor@users.noreply.github.com>
@setzor
setzor marked this pull request as ready for review October 27, 2026 06:08
@setzor
setzor merged commit 1a19e15 into main Oct 27, 2026
2 checks passed
@setzor
setzor deleted the vibe/shortcut-guards-emoji-sync-3d935b branch October 27, 2026 06:08

setzor commented Oct 27, 2026

Copy link
Copy Markdown
Owner Author

Self-review pass (CodeRabbit-style) over all session changes (PRs #47, #48, and this PR):

Fixes applied in 16b24f1:

  1. Shortcut guard covered too few modals (this PR's original guard only knew the app-settings overlay). Now also covers the notebook-settings dialog, the frosted prompt, and the context menu — previously t/arrows could navigate the journal behind those.
  2. Image alignment regression introduced in Fix image centering regression and guard cover-title edits per notebook #48: the new default margin: 1em auto (centering) overrode explicit data-text-align=left/right because those rules set only one margin side; both sides are now set per alignment.

Verified non-issues:

  • flushNow() return semantics: at boot autosave is null, so (await autosave?.flushNow()) === false is undefined === false → navigation proceeds. Correct.
  • applyNotebookMeta and the shelf rename flow share the same NotebookMeta objects — no double-mutation hazard.
  • createToolbar's selectionchange listener leak is pre-existing (single toolbar, never re-created) — left as-is.
  • contenteditable attribute fallback in shouldSkipShortcut is needed because happy-dom (and some WebViews) don't reflect isContentEditable reliably.

Checks: pnpm test (93 passed), pnpm typecheck, pnpm build all green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants