fix(macro): show a just-added Conditional's new condition and branch counts - #2151
Conversation
…counts The condition dialog and the branch pages edit the step in place. A step added this session is a class instance that $state does not proxy, and the list got the same object back, so the row kept its old label. Fixes #2147
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthrough
ChangesConditional command refresh
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The conditional display change is ready to merge after normal checks; no actionable risk was identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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. A rabbit checks the branch count twice, Comment |
1 participant
On a macro's page, a Conditional step added in the same session kept "(missing variable) is truthy" after its condition was saved in the dialog, and "Then: 0" after its Then page was left with a step added. The edit was saved; reopening the macro showed it. Not new in #2141 (the dialog builder did the same), but #2141 makes the branch pages part of the same flow.
The condition dialog and the branch pages edit the step in place, and
CommandListhanded that same object back to the list. A step loaded from data.json is a plain object, which$stateproxies, so the in-place edit re-rendered the row. A step added this session is aConditionalCommandclass instance, which$statedoes not proxy, and with the same reference handed back nothing re-rendered.updateCommandnow puts a copy in the list. Persistence is unchanged: the list is still snapshotted to plain data before it is saved.Checked in Obsidian 1.13.7: add a Conditional, set its condition to
mood, and the row reads "$mood is truthy" at once; add a Wait on its Then page, go back, and it reads "Then: 1".Tests
CommandList.conditionalcases with a freshConditionalCommand(a class instance, as an added step is): the label after the condition dialog saves, and the Then count after the branch page reports an edit. Both fail on master.pnpm run build-with-lint,pnpm run test(6620 passed) and.agents/run-e2epass.Fixes #2147
Note
Fix
CommandList.updateCommandso in-place Conditional edits re-renderAdds regression tests in CommandList.conditional.test.ts that mutate a
ConditionalCommandin place via edit callbacks and assert the summary and branch count update.updateCommandin CommandList.svelte now passes a shallow copy toreplaceById. The new object identity triggers reactivity for class-instance edits not proxied by$state.updateCommandnow persists a shallow copy of the command instead of the original reference.Macroscope summarized ecd49bd.
Summary by CodeRabbit