revert(gui): remove modal reload refactor series#1137
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThis PR removes the xstate-based ModalReloadController FSM machinery and replaces controller-driven modal reload orchestration with simpler direct re-rendering via display() calls across all affected modal components and choice builders. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d5b6c306b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| this.reloadController.destroy(); | ||
| this.resolvePromise(this.providers); |
There was a problem hiding this comment.
When onClose() is triggered while editing a provider (this.selectedProvider is set), the method now falls through to resolvePromise/super.onClose after calling this.open(). This closes the modal instead of returning to the provider list and can persist in-progress edits that users expected to discard (e.g., pressing Esc/X or Cancel-with-changes path), which is a behavior regression from the previous flow.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| this.selectedProvider = null; | ||
| this.reload(); | ||
| this.open(); | ||
| return; | ||
| } | ||
|
|
||
| this.reloadController.destroy(); | ||
| this.resolvePromise(this.providers); | ||
| super.onClose(); |
There was a problem hiding this comment.
🔴 Missing early return in AIAssistantProvidersModal.onClose() causes premature promise resolution and modal teardown
The revert removed the return statement from the if (this.selectedProvider) branch in onClose(). Previously (src/gui/AIAssistantProvidersModal.ts:388), when a user closed the modal while editing a provider, the code would set selectedProvider = null, reload to show the providers list, reopen the modal, and return — preventing resolvePromise and super.onClose() from executing. Now, after the reopen, execution falls through to this.resolvePromise(this.providers) and super.onClose(), which prematurely resolves the waitForClose promise (signaling the caller that the user is done) and tears down the modal. The caller at src/gui/AIAssistantSettingsModal.ts:68-72 will process the returned providers and reload its own UI while the providers modal is still supposed to be open.
(Refers to lines 374-383)
Was this helpful? React with 👍 or 👎 to provide feedback.
# [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)
|
🎉 This PR is included in version 2.12.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🚀 Release has been published: v2.12.0 |
1 participant
Revert the modal reload refactor series introduced in #1132, #1133, #1134, #1135, and #1136.
The combined result of that series was a more complex reload architecture built around the modal reload machine and a set of follow-up in-place UI updates. We do not want that direction in the codebase, so this PR removes the whole series rather than trying to preserve pieces of it.
This branch returns the repository to the exact tree from before #1132 landed. In practice that means:
xstatedependency that was only needed for this workValidation on this revert branch:
bun run testbun run build-with-lintFocused review note: the important property here is not the individual file diff, but that
HEADmatches the pre-series tree exactly.Summary by CodeRabbit
Chores
Refactor