Skip to content

fix(gui): reduce choice builder full-reload usage#1136

Merged
chhoumann merged 1 commit intomasterfrom
fsm-phase4-reduce-choicebuilder-reloads
Mar 5, 2026
Merged

fix(gui): reduce choice builder full-reload usage#1136
chhoumann merged 1 commit intomasterfrom
fsm-phase4-reduce-choicebuilder-reloads

Conversation

@chhoumann
Copy link
Owner

@chhoumann chhoumann commented Mar 5, 2026

Reduce remaining full modal reload usage in shared ChoiceBuilder open-file controls by rerendering only the file-opening section in place.

This is phase 4 of the FSM/reload stabilization stream after #1132, #1133, #1134, and #1135.

What changed:

  • ChoiceBuilder
    • addOpenFileSetting now supports context-aware in-place section updates
    • introduced local section container choiceBuilder__fileOpeningSection
    • open toggle and file-opening location changes rerender only that section
    • legacy fallback remains: when context is not provided, existing reload behavior is unchanged
  • TemplateChoiceBuilder and CaptureChoiceBuilder
    • switched to shared context-aware call: addOpenFileSetting(..., "created" / "captured")
    • removed duplicate conditional call pattern that depended on full modal reload
  • updated living ExecPlan with phase-4 progress, decisions, and evidence

Why:

  • open-file and location controls are high-frequency pivots that only affect a narrow subsection of UI.
  • rerendering only that section reduces modal churn and preserves user editing context more naturally.

Validation:

  • bun run test
  • bun run build-with-lint
  • Obsidian CLI smoke in dev vault:
    • obsidian vault=dev plugin:reload id=quickadd
    • obsidian vault=dev command id=quickadd:testQuickAdd
    • obsidian vault=dev dev:errors (no runtime errors)

Refs #1130


Open with Devin

Summary by CodeRabbit

  • Improvements
    • Enhanced file-opening settings rendering to eliminate unnecessary full modal reloads when toggling open-file options or changing file-opening locations in template and capture builders. File-opening settings now update in place for immediate feedback, providing a faster and more responsive experience when configuring file behavior.

@vercel
Copy link

vercel bot commented Mar 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
quickadd Ready Ready Preview Mar 5, 2026 7:12am

@chhoumann chhoumann marked this pull request as ready for review March 5, 2026 07:11
@coderabbitai
Copy link

coderabbitai bot commented Mar 5, 2026

📝 Walkthrough

Walkthrough

The PR implements context-aware in-place rendering for file-opening settings in ChoiceBuilder and its subclasses. The addOpenFileSetting method now accepts an optional contextLabel parameter; when provided, toggling the open-file option triggers a localized re-render instead of a full modal reload, maintaining backward compatibility with the original reload behavior when no context label is supplied.

Changes

Cohort / File(s) Summary
Planning Documentation
plans/issue-1130-fsm-modal-reload.md
Documents phase-4 migration approach for shared in-place rendering and generalized context-aware rendering via ChoiceBuilder.
Core Rendering Logic
src/gui/ChoiceBuilder/choiceBuilder.ts
Extends addOpenFileSetting with optional contextLabel parameter; adds private state field fileOpeningSectionContainer, and new private methods renderFileOpeningSection and addFileOpeningSettingsToContainer to support conditional in-place re-rendering instead of reload when context is provided.
Subclass Implementations
src/gui/ChoiceBuilder/captureChoiceBuilder.ts, src/gui/ChoiceBuilder/templateChoiceBuilder.ts
Updates call sites to pass target/context parameter ("captured" and "created" respectively) to addOpenFileSetting, replacing conditional logic that previously only added the setting when choice.openFile was true.

Possibly related PRs

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hopping through choice-builders with glee,
No reloads to slow us down, you'll see!
Context-aware renders in place,
Make the UI keep up its pace!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: reducing full-reload usage in the choice builder by implementing in-place section rerendering for file-opening controls.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fsm-phase4-reduce-choicebuilder-reloads

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

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

@cloudflare-workers-and-pages
Copy link

Deploying quickadd with  Cloudflare Pages  Cloudflare Pages

Latest commit: 7c7cc51
Status: ✅  Deploy successful!
Preview URL: https://942dbe94.quickadd.pages.dev
Branch Preview URL: https://fsm-phase4-reduce-choicebuil.quickadd.pages.dev

View logs

Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/gui/ChoiceBuilder/choiceBuilder.ts (1)

30-37: Clear stale file-opening container references on fallback/full-render paths.

fileOpeningSectionContainer can retain a detached element reference across redraw branches. Resetting it explicitly makes section-mode detection safer for future call paths.

♻️ Suggested hardening
 		this.reloadController = new ModalReloadController({
 			modalEl: this.modalEl,
 			contentEl: this.contentEl,
 			render: () => {
+				this.fileOpeningSectionContainer = null;
 				this.contentEl.empty();
 				this.display();
 			},
 		});
@@
-		if (!contextLabel) return;
+		if (!contextLabel) {
+			this.fileOpeningSectionContainer = null;
+			return;
+		}

Also applies to: 139-143

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/gui/ChoiceBuilder/choiceBuilder.ts` around lines 30 - 37, The
fileOpeningSectionContainer reference can persist a detached DOM node across
redraw branches; update the ModalReloadController render callback (where
reloadController is created) and the other fallback/full-render paths (the
branches around the display logic referenced at lines ~139-143) to explicitly
null/clear fileOpeningSectionContainer before repopulating the UI: inside the
render: () => { ... } and in the fallback/full-render branches set
this.fileOpeningSectionContainer = null (or empty container) prior to calling
this.contentEl.empty() / this.display() so section-mode detection always sees a
fresh container reference.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/gui/ChoiceBuilder/choiceBuilder.ts`:
- Around line 30-37: The fileOpeningSectionContainer reference can persist a
detached DOM node across redraw branches; update the ModalReloadController
render callback (where reloadController is created) and the other
fallback/full-render paths (the branches around the display logic referenced at
lines ~139-143) to explicitly null/clear fileOpeningSectionContainer before
repopulating the UI: inside the render: () => { ... } and in the
fallback/full-render branches set this.fileOpeningSectionContainer = null (or
empty container) prior to calling this.contentEl.empty() / this.display() so
section-mode detection always sees a fresh container reference.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 46b5caf5-10ba-41a1-8d32-f3e0718eb2dd

📥 Commits

Reviewing files that changed from the base of the PR and between a1a6271 and 7c7cc51.

📒 Files selected for processing (4)
  • plans/issue-1130-fsm-modal-reload.md
  • src/gui/ChoiceBuilder/captureChoiceBuilder.ts
  • src/gui/ChoiceBuilder/choiceBuilder.ts
  • src/gui/ChoiceBuilder/templateChoiceBuilder.ts

@chhoumann chhoumann merged commit 818272a into master Mar 5, 2026
5 checks passed
@chhoumann chhoumann deleted the fsm-phase4-reduce-choicebuilder-reloads branch March 5, 2026 07:18
chhoumann added a commit that referenced this pull request Mar 5, 2026
chhoumann added a commit that referenced this pull request Mar 5, 2026
* Revert "fix(gui): reduce choice builder reload churn (#1136)"

This reverts commit 818272a.

* Revert "fix(gui): reduce macro settings modal reload churn (#1135)"

This reverts commit a1a6271.

* Revert "fix(gui): reduce ai settings modal reload churn (#1134)"

This reverts commit ae0f7a1.

* Revert "test(gui): harden modal reload queue replay (#1133)"

This reverts commit 9373144.

* Revert "fix(gui): preserve modal edit position during settings reload (#1132)"

This reverts commit 11bda19.
github-actions bot pushed a commit that referenced this pull request Mar 5, 2026
# [2.12.0](2.11.0...2.12.0) (2026-03-05)

### Bug Fixes

* **capture:** preserve canvas tab indentation on configured writes ([#1125](#1125)) ([0a1578e](0a1578e))
* disallow capture targets with .base extension ([cb39ed4](cb39ed4))
* **field-suggestions:** opt-in inline values from fenced code blocks ([#1128](#1128)) ([8597905](8597905))
* **gui:** preserve modal edit position during settings reload ([#1132](#1132)) ([11bda19](11bda19))
* **gui:** reduce ai settings modal reload churn ([#1134](#1134)) ([ae0f7a1](ae0f7a1))
* **gui:** reduce choice builder reload churn ([#1136](#1136)) ([818272a](818272a))
* **gui:** reduce macro settings modal reload churn ([#1135](#1135)) ([a1a6271](a1a6271))
* harden existing-tab matching and document issue workflow ([#1108](#1108)) ([7b12d3b](7b12d3b))
* make template path resolution deterministic ([3297d54](3297d54))
* normalize capture title for non-markdown targets ([964d672](964d672))
* preserve capture-format spacing for insert-at-end ([#1119](#1119)) ([8bb8ed4](8bb8ed4))
* preserve explicit capture target file extensions ([57e43ff](57e43ff))
* preserve insert-at-end order for non-newline captures ([#1120](#1120)) ([e7cbbf2](e7cbbf2))
* resolve template file-name paths without duplicate default folder ([7bfd41b](7bfd41b))
* resolve vault-relative template paths using root folders ([81216de](81216de))

### Features

* add AI request logging API and reduce assistant log noise ([#1110](#1110)) ([2c36800](2c36800))
* automate docs version snapshot during release ([#1111](#1111)) ([1571846](1571846))
* **capture:** fully support capture into canvas cards ([#1124](#1124)) ([a53f889](a53f889))
* **cli:** add native QuickAdd Obsidian CLI handlers ([#1129](#1129)) ([8102d47](8102d47))
* **format:** support mapped VALUE suggester display text ([#1127](#1127)) ([b8ec56c](b8ec56c))
* **macro:** add editor cursor navigation commands ([101d5f6](101d5f6))
* support .base template files for template choices ([11e6490](11e6490))

### Reverts

* **gui:** remove modal reload refactor series ([#1137](#1137)) ([3ba1a73](3ba1a73)), closes [#1136](#1136) [#1135](#1135) [#1134](#1134) [#1133](#1133) [#1132](#1132)
@github-actions
Copy link

github-actions bot commented Mar 5, 2026

🎉 This PR is included in version 2.12.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant