feat(template): replace folder-location toggles with a single dropdown (#1131)#1344
Merged
Merged
Conversation
#1131) Consolidate the Template choice builder's four interdependent folder toggles (Create in folder / Choose folder when creating / Include subfolders / Create in same folder as active file) into one "New note location" dropdown over the existing persisted booleans — no schema change, engines untouched, forward/backward compatible. A pure helper (folderMode.ts) maps the four booleans to four mutually-exclusive modes, mirroring TemplateChoiceEngine.getFolderPath() precedence exactly (pinned by a 16-combo oracle test). Selecting a mode writes the canonical flags; switching modes hides but never deletes the configured folder list (fixes the "my config disappeared" complaint). "Include subfolders" is scoped to the one mode where it has an effect, and chooseFromSubfolders is reset outside that mode so it stays inert in TemplateInsertEngine's move-offer. Closes #1131
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughReplaces the multi-boolean folder destination toggles in ChangesFolder Mode Dropdown Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Sequence DiagramsequenceDiagram
participant User as User (TemplateChoiceForm)
participant Component as TemplateChoiceForm.svelte
participant Derive as deriveFolderMode
participant Apply as applyFolderMode
participant Config as choice.folder
User->>Component: opens form with existing config
Component->>Derive: derive current FolderMode
Derive-->>Component: return canonical mode
Component->>Component: render dropdown with current mode
User->>Component: selects new location mode
Component->>Apply: apply selected mode to config
Apply-->>Config: return updated TemplateFolderConfig
Config-->>Component: update choice.folder
Component->>Component: conditionally show/hide folder list, warning, subfolders toggle
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
Deploying quickadd with
|
| Latest commit: |
cbe3d6f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e0a28843.quickadd.pages.dev |
| Branch Preview URL: | https://chhoumann-1131-folder-select.quickadd.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/gui/ChoiceBuilder/folderMode.ts (2)
87-102: ⚡ Quick winUse camelCase for exported constants in this TypeScript module.
At Line 87 and Line 94, rename
FOLDER_MODE_OPTIONS/FOLDER_MODE_DESCRIPTIONSto camelCase and update call sites.Suggested rename diff
-export const FOLDER_MODE_OPTIONS: { value: FolderMode; label: string }[] = [ +export const folderModeOptions: { value: FolderMode; label: string }[] = [ @@ -export const FOLDER_MODE_DESCRIPTIONS: Record<FolderMode, string> = { +export const folderModeDescriptions: Record<FolderMode, string> = {As per coding guidelines:
**/*.{ts,tsx}: Use camelCase for variables and functions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/gui/ChoiceBuilder/folderMode.ts` around lines 87 - 102, Rename the exported constants FOLDER_MODE_OPTIONS and FOLDER_MODE_DESCRIPTIONS to camelCase (folderModeOptions and folderModeDescriptions respectively) to comply with the TypeScript coding guidelines for variables and constants. After renaming, locate and update all call sites throughout the codebase that reference these constants to use the new camelCase names.Source: Coding guidelines
1-1: ⚡ Quick winRename this utility module to kebab-case.
src/gui/ChoiceBuilder/folderMode.tsshould follow the repo utility naming rule; please rename it tofolder-mode.tsand update imports accordingly.As per coding guidelines:
**/*: Use kebab-case for directories and utilities.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/gui/ChoiceBuilder/folderMode.ts` at line 1, Rename the utility module folderMode.ts to folder-mode.ts to comply with the kebab-case naming convention for utilities in the repository. After renaming the file, search the codebase for all import statements that reference the old filename and update them to import from folder-mode.ts instead of folderMode.ts.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/gui/ChoiceBuilder/folderMode.ts`:
- Around line 87-102: Rename the exported constants FOLDER_MODE_OPTIONS and
FOLDER_MODE_DESCRIPTIONS to camelCase (folderModeOptions and
folderModeDescriptions respectively) to comply with the TypeScript coding
guidelines for variables and constants. After renaming, locate and update all
call sites throughout the codebase that reference these constants to use the new
camelCase names.
- Line 1: Rename the utility module folderMode.ts to folder-mode.ts to comply
with the kebab-case naming convention for utilities in the repository. After
renaming the file, search the codebase for all import statements that reference
the old filename and update them to import from folder-mode.ts instead of
folderMode.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9ed951d1-c448-42cd-a184-1b7256e3fbac
📒 Files selected for processing (7)
src/gui/ChoiceBuilder/TemplateChoiceForm.sveltesrc/gui/ChoiceBuilder/TemplateChoiceForm.test.tssrc/gui/ChoiceBuilder/folderMode.test.tssrc/gui/ChoiceBuilder/folderMode.tssrc/styles.csssrc/types/choices/ITemplateChoice.tssrc/types/choices/TemplateChoice.ts
Adversarial review (3 Codex reviewers + CodeRabbit) findings: - Preserve chooseFromSubfolders across mode switches instead of resetting it outside "specified". The reset caused a real regression vs the old form: switching specified -> other -> specified silently dropped the "Include subfolders" preference. applyFolderMode now only sets the mode-discriminant flags; folders[] and chooseFromSubfolders are always preserved (the old form hid but never cleared them). No new flag combination is produced, so the other consumers (TemplateInsertEngine, collectChoiceRequirements) see no regression. - Reword the empty-"specific folder" warning: with no active file the engine prompts (or uses root), it does not always use the active file's folder. - Add a back-reference comment in TemplateChoiceEngine.getFolderPath() pointing to folderMode.ts so the precedence mirror is kept in sync. - camelCase the exported folderModeOptions/folderModeDescriptions (CodeRabbit). - Update docs/docs/Choices/TemplateChoice.md to the dropdown model. Skipped: CodeRabbit's kebab-case file-rename nit — folderMode.ts matches its camelCase siblings (choiceBuilder.ts, captureChoiceBuilder.ts, templateChoiceFormProps.svelte.ts); renaming would make it inconsistent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1131.
Problem
The Template choice builder's Location section used 4 interdependent toggles (Create in folder · Choose folder when creating a new note · Include subfolders · Create in same folder as active file) whose visibility depended on each other. New users found this confusing — and turning on the last toggle made the folder config above it vanish. Confirmed live in the dev vault (toggling Create in same folder as active file ON removes the folder input and two other toggles).
Solution
One "New note location" dropdown with four mutually-exclusive modes:
It matches the form's existing pattern (the "If the target file already exists" row is already a dropdown) and is obsidian-native (
<select>).How it works — no schema change
The dropdown is a derived view over the existing four persisted booleans (
folder.{enabled, folders, chooseWhenCreatingNote, createInSameFolderAsActiveFile, chooseFromSubfolders}):src/gui/ChoiceBuilder/folderMode.ts) maps booleans → mode (deriveFolderMode) and mode → canonical booleans (applyFolderMode).deriveFolderModemirrorsTemplateChoiceEngine.getFolderPath()precedence exactly — pinned by a 16-combination oracle test, so the dropdown always shows the mode that will actually run (legacy choices with multiple flags set open on their true mode).Include subfoldersis scoped to the one mode where the engine honours it;chooseFromSubfoldersis reset outside that mode so it stays inert inTemplateInsertEngine's move-offer.Zero migration · forward/backward compatible · engines untouched · package export/import unaffected. Existing
data.jsonworks as-is; an older QuickAdd build still reads the booleans.Empty-folder guard
"In a specific folder" with an empty list previously fell through to the active file's folder silently. A required inline warning now flags this.
Scope (maintainer-confirmed)
captureTostring +captureToActiveFile) and does not have this bug.Verification
folderMode.test.ts: 16-combo precedence oracle, round-trip, residue-reset; rewrittenTemplateChoiceForm.test.tskeeps the [FEATURE REQUEST] UX Improvement: Modal completely refreshes on button clicks, causing loss of edit position #1130 no-remount guard).tsc+ ESLint clean; production build OK.data.json(active-file → promptresetscreateInSameFolderAsActiveFile;prompt → specified+ folder + subfolders persists).Migration / release impact
None. User-facing UI only;
feat→ minor release.Summary by CodeRabbit