Skip to content

fix(prompt): drop Peek when a prompt opens over another plugin's modal - #2239

Merged
chhoumann merged 2 commits into
masterfrom
issue-2234
Oct 13, 2026
Merged

chhoumann merged 2 commits into
masterfrom
issue-2234

Conversation

@chhoumann

@chhoumann chhoumann commented Oct 13, 2026 •

Copy link
Copy Markdown
Owner

Fixes #2234. Related to #2235 and #2238.

Summary

When a QuickAdd run starts while another dialog is open, such as Full Calendar Remastered's New Event editor, the QuickAdd prompt opens on top of that dialog. Before this change, the prompt still offered Peek at note and Ctrl/Cmd+Shift+E. Peeking there:

  • hid the prompt and showed the other dialog, not the note,
  • moved keyboard focus to the editor behind that dialog's dimmed backdrop,
  • parked the run behind a "QuickAdd is waiting" chip that overlapped the dialog.

The prompt now omits Peek while another modal is open in the same window. The shortcut does nothing there. Over a plain note, Peek works as before.

How it works

  • A CSS rule in src/styles.css hides .qa-peek-button while body has a direct .modal-container child without .quickAddModal.
  • InputPromptPeek.peek() returns early on the same condition. This covers the shortcut on the text prompt and on the one-page form.

The rule is evaluated live, not once when the prompt opens. On phones, Obsidian runs a command palette pick while the palette is still animating closed, and its container is still in the DOM. A one-time check at open hid Peek on every palette run on a phone. With the live rule, Peek is hidden for about 250 ms and appears once the palette detaches. The Cancel and Ok buttons do not move.

Why keep Peek and only hide it here

Peek is intentional. It shipped in 2.23.0 (#1688) so a user can read or select text in the note without losing a draft. The reporter may have upgraded from a release before 2.23.0, which explains "it used to work". Peek's design assumes the prompt sits over the workspace, and supportsPeek() already withholds it from settings and builder prompts for that reason. Over a third-party dialog, the note is behind a modal backdrop, so Peek cannot show it, and Insert selection reads the editor selection the user cannot make. Hiding Peek there removes a trap and costs nothing a user could use. Removing Peek everywhere would drop a working feature for the common case to fix a rare one.

Reproduction

Obsidian 1.13.7, Full Calendar Remastered 0.13.6, a capture choice with format Person: {{VALUE:Name}} that captures to the active file:

  1. Open a note, then run Full Calendar: New Event.
  2. With focus in Event Title, run the QuickAdd choice as a command.
  3. Click Peek at note or press Ctrl/Cmd+Shift+E.

Before: the prompt is hidden, focus moves to .cm-content outside every modal, and the chip overlaps the New Event dialog. After: the prompt has no Peek button, the shortcut does nothing, and submitting captures to the note and returns focus to Event Title.

QuickAdd hotkeys and the command palette cannot fire while Full Calendar's dialog is open, because Obsidian gives a modal's keymap scope no parent (checked in both 1.13.7 and 1.14.4). A real user reaches this state with a command run that bypasses hotkeys, or by starting QuickAdd first.

Not covered here

  • The reporter says the run "immediately" suspends. I did not reproduce an automatic peek. The only Peek triggers are the button and the shortcut. One way to reproduce it: if a choice's own hotkey is Ctrl/Cmd+Shift+E, holding it past key autorepeat (660 ms on the test display) peeks the new prompt at once. This PR does not change that.
  • fix(peek): restack a resumed prompt above modals opened during the peek #2238 fixes a related path: peek over a note, open another dialog, then click Return, and the prompt stays under that dialog.
  • The [BUG] #2235 thread found two follow-ups in that path, and neither is fixed here. Once a dialog opens during a peek, Ctrl/Cmd+Shift+E cannot return, because the dialog's scope is above the peek's return scope. The chip's Return button still works. On Android, after Return, Back closes the other dialog under the prompt.

Verification

  • New native spec tests/e2e/peek-over-plugin-modal.test.ts: 2 passed on this branch. Against the unfixed source, it fails with peekVisible: true over the other modal.
  • Full native suite (.agents/run-e2e): 452 passed, 24 skipped.
  • pnpm run test and pnpm run lint pass. The two new unit tests fail without the fix (expected true to be false on isPeeking()).
  • In real Obsidian with real Full Calendar Remastered on desktop: Peek is visible on a plain note when the run starts from the palette or from the QuickAdd picker, and hidden over the New Event dialog on the text prompt and on the one-page form. In phone emulation, a run from the palette shows Peek after the palette detaches.

Release impact

User-visible behavior change limited to prompts opened over another modal. No settings or migrations. The docs page Controlling prompts now says when Peek does not appear.

Note

Make InputPromptPeek.peek a no-op when a prompt opens over another plugin's modal

  • Adds an early return in InputPromptPeek.peek when the prompt container sits over another modal container, checked by the new isOverAnotherModal utility in InputPromptPeek.ts
  • Hides Peek buttons with a CSS rule in styles.css whenever the document body has a direct non-QuickAdd modal container
  • Updates the Peek documentation in ControllingPrompts.md to note that prompts opened over another dialog do not offer Peek
  • Adds regression tests for the input prompt and one-page input form, plus end-to-end coverage in peek-over-plugin-modal.test.ts that the shortcut is ignored and submission still works in the overlay case
  • Behavioral Change: isOverAnotherModal treats any direct modal container child of the document body (excluding QuickAdd's own) as an overlay; a plugin using a non-standard container structure could still allow Peek

Macroscope summarized 7ad7121.

Summary by CodeRabbit

  • Bug Fixes
    • Peek is now unavailable while another plugin’s modal is open. The Peek button is hidden in this situation, and the Peek shortcut won’t start a session. Prompts remain usable, and QuickAdd modals are not affected.
  • Documentation
    • Clarified that Peek is unavailable when a prompt opens over another dialog.

A run started while another plugin's dialog is open, such as Full
Calendar Remastered's event editor, opens its prompt on top of that
dialog. Peek then hid the prompt, showed the dialog instead of the note,
moved focus to the editor behind the dialog's backdrop, and parked the
run behind a "QuickAdd is waiting" chip.

A CSS rule hides the Peek button while a non-QuickAdd modal container is
in the document, and peek() checks the same containers so the
Ctrl/Cmd+Shift+E shortcut does nothing there. The rule is live, so a
command palette still animating closed on a phone hides Peek only until
it detaches.

Fixes #2234
@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e2f49648-e4d6-4af2-9e27-c93f0640cd0c
📥 Commits

Reviewing files that changed from the base of the PR and between 45cd3fb and 7ad7121.

📒 Files selected for processing: 6
  • docs/src/content/docs/docs/ControllingPrompts.md
  • src/gui/GenericInputPrompt/GenericInputPrompt.peek.test.ts
  • src/gui/promptPeek/InputPromptPeek.ts
  • src/preflight/OnePageInputModal.peek.test.ts
  • src/styles.css
  • tests/e2e/peek-over-plugin-modal.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

Peek now declines to start when another plugin modal is open. The prompt’s Peek button is hidden in that condition. Unit and end-to-end tests cover the behavior, and the documentation reflects the additional restriction.

Changes: Peek behavior over plugin modals

Layer / File(s) Summary
Modal detection and peek guard
src/gui/promptPeek/InputPromptPeek.ts, src/styles.css, src/gui/GenericInputPrompt/GenericInputPrompt.peek.test.ts, src/preflight/OnePageInputModal.peek.test.ts
Peek checks for a direct child modal container other than a QuickAdd modal. The CSS hides the Peek button in this case. Unit tests cover button clicks, the shortcut, and prompt completion.
End-to-end coverage and availability note
tests/e2e/peek-over-plugin-modal.test.ts, docs/src/content/docs/docs/ControllingPrompts.md
End-to-end tests cover Peek visibility over a note and a plugin modal, and verify prompt submission while the modal remains open. The documentation adds another dialog to the conditions that disable Peek.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 7ad71

No merge-blocking issue was established for the change. Normal checks remain appropriate before merging.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Linked Issues check Warning Active directly linked issue #2234 requires QuickAdd hooks to execute inline format syntax and input prompts sequentially while focus remains inside an active third-party modal, without manual interve… Implement the #2234 execution behavior for inline format syntax and input prompts inside an active third-party modal. Add a regression test that starts with focus in the third-party modal and verifies sequential execution without a manual P…
Docstring Coverage Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. (2 skipped: 2… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 3 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: disabling Peek when a prompt opens over another plugin's modal.
Out of Scope Changes check Passed The changed CSS, InputPromptPeek guard, documentation, unit tests, and end-to-end test all address the modal/Peek interaction described in #2234. The tests and documentation support that behavior. N…
Full details: Linked Issues check

Explanation

Active directly linked issue #2234 requires QuickAdd hooks to execute inline format syntax and input prompts sequentially while focus remains inside an active third-party modal, without manual intervention or tab-context changes. This pull request only hides Peek and makes InputPromptPeek.peek() return early when another modal exists. Its description explicitly leaves the reported suspension and inline-execution behavior unchanged. The new tests verify Peek suppression and a scripted submission, but they do not establish restoration of the requested hook behavior.

Resolution

Implement the #2234 execution behavior for inline format syntax and input prompts inside an active third-party modal. Add a regression test that starts with focus in the third-party modal and verifies sequential execution without a manual Peek action or tab-context change.

Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit taps the Peek button with care,
But sees another modal waiting there.
The prompt stays ready; “Ada” finds its way,
While Peek sits quietly until another day.
The rabbit hops, content with the change.

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying quickadd with  Cloudflare Pages  Cloudflare Pages

Latest commit: 7ad7121
Status: ✅  Deploy successful!
Preview URL: https://e274d8df.quickadd.pages.dev
Branch Preview URL: https://issue-2234.quickadd.pages.dev

View logs

@chhoumann
chhoumann marked this pull request as ready for review October 13, 2026 20:38
@chhoumann

Copy link
Copy Markdown
Owner Author

Independent verification: PASS

  • Head: 7ad712121eed215610dc88c4d3159390b6135133
  • Base: 45cd3fb030ab4d2c1b63ae8edad4f5d0f1180258 (master)

Setup: Obsidian 1.13.7 on Linux, isolated e2e vault on a private Xvfb, Full Calendar Remastered 0.13.6 release build with a local calendar. Two capture choices write to the active note at the cursor: a text prompt with Person: {{VALUE:Name}}, and a one-page form (onePageInput: always) with Person: {{VALUE:Name}}, Phone: {{VALUE:Phone}}. Keys and clicks were real X input through xdotool. Peek visibility was read with getClientRects().length > 0. Each build was rebuilt and loaded with obsidian:e2e -- --reload quickadd:list. The head instance had the new :has() rule in its loaded stylesheets and the parent instance did not.

Check Parent 45cd3fb Head 7ad7121
1a. Text prompt over FCR New Event: Peek button Visible Hidden
1a. Real Ctrl+Shift+E Peeks. Prompt hidden, New Event in front, focus on .cm-content outside every modal, "QuickAdd is waiting" chip over the dialog No-op. Prompt stays, focus stays in its field, no chip, field still empty
1a. Type "Ada", Enter After chip Return: note is Person: Ada, focus back in .ofc-premium-title-input Note is Person: Ada, focus back in .ofc-premium-title-input
1b. One-page form over FCR: Peek button Visible Hidden
1b. Real Ctrl+Shift+E Peeks, same symptoms as 1a No-op
1b. Fill Ada / 555, Ctrl+Enter After chip Return: Person: Ada, Phone: 555, focus back in FCR title Person: Ada, Phone: 555, focus back in FCR title
2. Plain note, run from Ctrl+P palette Peek visible. Click shows the chip, Return restores draft "Grace", submit writes Person: Grace Same. The Ctrl+Shift+E round trip also works
3. Phone emulation (400x860, is-phone), tap in palette Peek visible the whole time Peek hidden while the palette is still attached, shown about 270 ms after the tap when it detaches. Cancel, Ok and the modal height do not move. Peek and Return work
Native spec peek-over-plugin-modal.test.ts 1 of 2 fails (peekVisible: true) with the head spec against parent src 2 passed
The 2 new unit tests Both fail (expected true to be false) against parent src 13 of 13 pass in both files

Extra checks on head, all as expected:

  • Phone emulation over FCR New Event: Peek hidden, shortcut inert, capture lands, focus returns to the FCR title.
  • On a phone, a run from the QuickAdd picker (Run QuickAdd) behaves like a run from the palette.

The screenshots look right. Over FCR, head shows only Cancel/Ok (or Cancel/Submit), right-aligned, with no gap where Peek was.

Notes, none blocking:

  • Pre-existing, not from this PR: on a phone, the compact peek chip covers the note's inline title. Parent looks the same.
  • Test environment only: the app reload for phone emulation dropped FCR from community-plugins.json, so I re-enabled it before the phone FCR check.

@chhoumann
chhoumann merged commit e17ee7c into master Oct 13, 2026
16 checks passed
@chhoumann chhoumann mentioned this pull request Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]

1 participant