-
-
Notifications
You must be signed in to change notification settings - Fork 187
fix(gui): preserve choice-builder edit position by converting the ChoiceBuilder trio to Svelte (#1130) #1330
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
22b287a
feat(gui): add shared Svelte primitives for ChoiceBuilder conversion …
chhoumann 4c81b6a
feat(gui): convert TemplateChoiceBuilder to a Svelte form (#1130)
chhoumann 8b541ec
feat(gui): convert CaptureChoiceBuilder to a Svelte form (#1130)
chhoumann bb86217
refactor(gui): reduce ChoiceBuilder base to a Svelte mount shell (#1130)
chhoumann d47c659
fix(gui): keep choice-builder forms reactive for new choices + defaul…
chhoumann 056b6d5
Merge remote-tracking branch 'origin/master' into chhoumann/1130-moda…
chhoumann 841c7f6
fix(gui): address PR review feedback (#1130)
chhoumann 6febe11
Merge remote-tracking branch 'origin/master' into chhoumann/1130-moda…
chhoumann File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,178 @@ | ||
| <script lang="ts"> | ||
| import type { App } from "obsidian"; | ||
| import type QuickAdd from "../../main"; | ||
| import type ICaptureChoice from "../../types/choices/ICaptureChoice"; | ||
| import { getTemplateFile } from "../../utilityObsidian"; | ||
| import { FormatSyntaxSuggester } from "../suggesters/formatSyntaxSuggester"; | ||
| import SettingItem from "../components/SettingItem.svelte"; | ||
| import Toggle from "../components/Toggle.svelte"; | ||
| import Dropdown from "../components/Dropdown.svelte"; | ||
| import ChoiceNameHeader from "./components/ChoiceNameHeader.svelte"; | ||
| import ValidatedInput from "./components/ValidatedInput.svelte"; | ||
| import FormatPreviewField from "./components/FormatPreviewField.svelte"; | ||
| import AppendLinkSetting from "./components/AppendLinkSetting.svelte"; | ||
| import OpenFileSetting from "./components/OpenFileSetting.svelte"; | ||
| import FileOpeningSetting from "./components/FileOpeningSetting.svelte"; | ||
| import OnePageOverrideSetting from "./components/OnePageOverrideSetting.svelte"; | ||
| import CaptureTargetSetting from "./components/CaptureTargetSetting.svelte"; | ||
| import WritePositionSetting from "./components/WritePositionSetting.svelte"; | ||
|
|
||
| /** | ||
| * Reactive replacement for CaptureChoiceBuilder.display(). Every conditional row | ||
| * (capture target, create-if-missing, write position + insert-after/before fields, | ||
| * append link, file opening) is an {#if} over the $state choice proxy, so toggling | ||
| * a control updates in place — no contentEl.empty()/display() teardown (#1130). | ||
| */ | ||
| let { | ||
| choice = $bindable(), | ||
| app, | ||
| plugin, | ||
| }: { | ||
| choice: ICaptureChoice; | ||
| app: App; | ||
| plugin: QuickAdd; | ||
| } = $props(); | ||
|
|
||
| const templateFilePaths = $derived( | ||
| plugin.getTemplateFiles().map((f) => f.path), | ||
| ); | ||
| const formatSuggesters = [ | ||
| (el: HTMLInputElement | HTMLTextAreaElement) => | ||
| new FormatSyntaxSuggester(app, el, plugin), | ||
| ]; | ||
|
|
||
| function validateTemplate(raw: string): boolean | string { | ||
| const value = raw.trim(); | ||
| if (!value) return true; | ||
| // Resolve like the engine does at run time rather than requiring | ||
| // suggestion-list membership (templates outside the configured folders are | ||
| // valid). Mirrors templateChoiceBuilder (master #1170/#1325). | ||
| return getTemplateFile(app, value) !== null || "Template not found"; | ||
| } | ||
|
|
||
| const selectionOptions = [ | ||
| { value: "", label: "Follow global setting" }, | ||
| { value: "enabled", label: "Use selection" }, | ||
| { value: "disabled", label: "Ignore selection" }, | ||
| ]; | ||
| const selectionOverride = $derived( | ||
| typeof choice.useSelectionAsCaptureValue === "boolean" | ||
| ? choice.useSelectionAsCaptureValue | ||
| ? "enabled" | ||
| : "disabled" | ||
| : "", | ||
| ); | ||
|
|
||
| function onSelectionChange(value: string) { | ||
| if (value === "") { | ||
| choice.useSelectionAsCaptureValue = undefined; | ||
| return; | ||
| } | ||
| choice.useSelectionAsCaptureValue = value === "enabled"; | ||
| } | ||
|
|
||
| function onTemplaterAfterCaptureChange(value: boolean) { | ||
| if (!choice.templater) choice.templater = {}; | ||
| choice.templater.afterCapture = value ? "wholeFile" : "none"; | ||
| } | ||
| </script> | ||
|
|
||
| <ChoiceNameHeader bind:name={choice.name} {app} /> | ||
|
|
||
| <SettingItem name="Location" heading /> | ||
| <CaptureTargetSetting bind:choice {app} {plugin} /> | ||
|
|
||
| {#if !choice.captureToActiveFile} | ||
| <SettingItem name="Create file if it doesn't exist"> | ||
| {#snippet control()} | ||
| <Toggle bind:checked={choice.createFileIfItDoesntExist.enabled} /> | ||
| {/snippet} | ||
| </SettingItem> | ||
|
|
||
| {#if choice.createFileIfItDoesntExist.enabled} | ||
| <SettingItem name="Create file with given template."> | ||
| {#snippet control()} | ||
| <Toggle | ||
| bind:checked={choice.createFileIfItDoesntExist.createWithTemplate} | ||
| /> | ||
| {/snippet} | ||
| </SettingItem> | ||
| <ValidatedInput | ||
| value={choice.createFileIfItDoesntExist.template} | ||
| placeholder="Template path" | ||
| disabled={!choice.createFileIfItDoesntExist.createWithTemplate} | ||
| {app} | ||
| suggestions={templateFilePaths} | ||
| maxSuggestions={50} | ||
| validator={validateTemplate} | ||
| ariaLabel="Template path" | ||
| onChange={(value) => | ||
| (choice.createFileIfItDoesntExist.template = value.trim())} | ||
| /> | ||
| {/if} | ||
| {/if} | ||
|
|
||
| <SettingItem name="Position" heading /> | ||
| <WritePositionSetting bind:choice {app} {plugin} /> | ||
|
|
||
| <SettingItem name="Linking" heading /> | ||
| <AppendLinkSetting bind:appendLink={choice.appendLink} fileLabel="captured" /> | ||
|
|
||
| <SettingItem name="Content" heading /> | ||
| <SettingItem name="Task" desc="Formats the value as a task."> | ||
| {#snippet control()} | ||
| <Toggle bind:checked={choice.task} /> | ||
| {/snippet} | ||
| </SettingItem> | ||
|
|
||
| <SettingItem name="Capture format" desc="Set the format of the capture."> | ||
| {#snippet control()} | ||
| <Toggle bind:checked={choice.format.enabled} /> | ||
| {/snippet} | ||
| </SettingItem> | ||
| <FormatPreviewField value={choice.format.format} {app} {plugin} /> | ||
| <ValidatedInput | ||
| inputKind="textarea" | ||
| bind:value={choice.format.format} | ||
| placeholder="Format" | ||
| disabled={!choice.format.enabled} | ||
| required={choice.format.enabled} | ||
| requiredMessage="Capture format is required when enabled" | ||
| makeSuggesters={formatSuggesters} | ||
| ariaLabel="Format" | ||
| /> | ||
|
|
||
| <SettingItem name="Behavior" heading /> | ||
| {#if !choice.captureToActiveFile} | ||
| <OpenFileSetting bind:openFile={choice.openFile} description="Open the captured file." /> | ||
| {#if choice.openFile} | ||
| <FileOpeningSetting bind:fileOpening={choice.fileOpening} contextLabel="captured" /> | ||
| {/if} | ||
| {/if} | ||
|
|
||
| <SettingItem | ||
| name="Use editor selection as default value" | ||
| desc={"Controls whether this Capture uses the current editor selection as {{VALUE}}. Does not affect {{SELECTED}}."} | ||
| > | ||
| {#snippet control()} | ||
| <Dropdown | ||
| value={selectionOverride} | ||
| options={selectionOptions} | ||
| onchange={onSelectionChange} | ||
| /> | ||
| {/snippet} | ||
| </SettingItem> | ||
|
|
||
| <SettingItem | ||
| name="Run Templater on entire destination file after capture" | ||
| desc="Advanced / legacy: this executes any <% %> anywhere in the destination file (including inside code blocks)." | ||
| > | ||
| {#snippet control()} | ||
| <Toggle | ||
| checked={choice.templater?.afterCapture === "wholeFile"} | ||
| onchange={onTemplaterAfterCaptureChange} | ||
| /> | ||
| {/snippet} | ||
| </SettingItem> | ||
|
|
||
| <OnePageOverrideSetting bind:onePageInput={choice.onePageInput} /> | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,157 @@ | ||
| import { describe, expect, it, vi } from "vitest"; | ||
|
|
||
| vi.mock("obsidian-dataview", () => ({ getAPI: vi.fn() })); | ||
|
|
||
| import { App } from "obsidian"; | ||
| import { fireEvent, render } from "@testing-library/svelte"; | ||
| import { flushSync } from "svelte"; | ||
| import type QuickAdd from "../../main"; | ||
| import type ICaptureChoice from "../../types/choices/ICaptureChoice"; | ||
| import CaptureChoiceForm from "./CaptureChoiceForm.svelte"; | ||
| import { createCaptureChoiceFormProps } from "./captureChoiceFormProps.svelte"; | ||
| import { CaptureChoice } from "../../types/choices/CaptureChoice"; | ||
|
|
||
| function captureChoice(): ICaptureChoice { | ||
| return { | ||
| id: "c1", | ||
| name: "My Capture", | ||
| type: "Capture", | ||
| command: false, | ||
| captureTo: "Inbox.md", | ||
| captureToActiveFile: false, | ||
| captureToCanvasNodeId: "", | ||
| activeFileWritePosition: "cursor", | ||
| createFileIfItDoesntExist: { | ||
| enabled: false, | ||
| createWithTemplate: false, | ||
| template: "", | ||
| }, | ||
| format: { enabled: false, format: "" }, | ||
| prepend: false, | ||
| appendLink: false, | ||
| task: false, | ||
| insertAfter: { | ||
| enabled: false, | ||
| after: "", | ||
| insertAtEnd: false, | ||
| considerSubsections: false, | ||
| createIfNotFound: false, | ||
| createIfNotFoundLocation: "top", | ||
| }, | ||
| insertBefore: { | ||
| enabled: false, | ||
| before: "", | ||
| createIfNotFound: false, | ||
| createIfNotFoundLocation: "top", | ||
| }, | ||
| newLineCapture: { enabled: false, direction: "below" }, | ||
| openFile: false, | ||
| fileOpening: { | ||
| location: "tab", | ||
| direction: "vertical", | ||
| mode: "default", | ||
| focus: true, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| const plugin = { | ||
| getTemplateFiles: () => [], | ||
| settings: { choices: [] }, | ||
| } as unknown as QuickAdd; | ||
|
|
||
| function settingNames(container: HTMLElement): string[] { | ||
| return Array.from(container.querySelectorAll(".setting-item-name")).map( | ||
| (el) => el.textContent ?? "", | ||
| ); | ||
| } | ||
|
|
||
| function selectUnderSetting( | ||
| container: HTMLElement, | ||
| name: string, | ||
| ): HTMLSelectElement { | ||
| const item = Array.from(container.querySelectorAll(".setting-item")).find( | ||
| (el) => el.querySelector(".setting-item-name")?.textContent === name, | ||
| ); | ||
| return item?.querySelector("select") as HTMLSelectElement; | ||
| } | ||
|
|
||
| function mountForm() { | ||
| const props = createCaptureChoiceFormProps({ | ||
| choice: captureChoice(), | ||
| app: new App(), | ||
| plugin, | ||
| }); | ||
| const result = render(CaptureChoiceForm, { | ||
| props: { choice: props.choice, app: props.app, plugin: props.plugin }, | ||
| }); | ||
| return { ...result, props }; | ||
| } | ||
|
|
||
| describe("CaptureChoiceForm", () => { | ||
| it("reveals insert-after / insert-before fields by write position, mutually exclusive, without remounting", async () => { | ||
| const { container } = mountForm(); | ||
| const headerBefore = container.querySelector(".choiceNameHeaderButton"); | ||
| expect(settingNames(container)).not.toContain("Insert after"); | ||
|
|
||
| const select = selectUnderSetting(container, "Write position"); | ||
| await fireEvent.change(select, { target: { value: "after" } }); | ||
| flushSync(); | ||
| expect(settingNames(container)).toContain("Insert after"); | ||
| expect(settingNames(container)).not.toContain("Insert before"); | ||
|
|
||
| await fireEvent.change(select, { target: { value: "before" } }); | ||
| flushSync(); | ||
| expect(settingNames(container)).toContain("Insert before"); | ||
| expect(settingNames(container)).not.toContain("Insert after"); | ||
|
|
||
| // No full remount across all those conditional changes (#1130). | ||
| expect(container.querySelector(".choiceNameHeaderButton")).toBe( | ||
| headerBefore, | ||
| ); | ||
| }); | ||
|
|
||
| it("hides the create/open/file-opening sections when capturing to the active file", () => { | ||
| const { container, props } = mountForm(); | ||
| expect(settingNames(container)).toContain("Create file if it doesn't exist"); | ||
|
|
||
| props.choice.captureToActiveFile = true; | ||
| flushSync(); | ||
| const names = settingNames(container); | ||
| expect(names).not.toContain("Create file if it doesn't exist"); | ||
| expect(names).not.toContain("Open"); | ||
| }); | ||
|
|
||
| it("stays reactive for a freshly created class-instance choice (add-new flow)", () => { | ||
| // createChoice() returns `new CaptureChoice()` — a class instance. Svelte's | ||
| // proxy() leaves class instances un-proxied, so the form props factory must | ||
| // plain-clone the choice or conditional rows won't react. This test fails if | ||
| // the structuredClone in createCaptureChoiceFormProps is removed. | ||
| const props = createCaptureChoiceFormProps({ | ||
| choice: new CaptureChoice("New Capture"), | ||
| app: new App(), | ||
| plugin, | ||
| }); | ||
| const { container } = render(CaptureChoiceForm, { | ||
| props: { choice: props.choice, app: props.app, plugin: props.plugin }, | ||
| }); | ||
| expect(settingNames(container)).toContain("Create file if it doesn't exist"); | ||
|
|
||
| props.choice.captureToActiveFile = true; | ||
| flushSync(); | ||
| expect(settingNames(container)).not.toContain( | ||
| "Create file if it doesn't exist", | ||
| ); | ||
| }); | ||
|
|
||
| it("persists write-position edits onto the form proxy (snapshot reflects them)", async () => { | ||
| const { container, props } = mountForm(); | ||
| const select = selectUnderSetting(container, "Write position"); | ||
| await fireEvent.change(select, { target: { value: "before" } }); | ||
| flushSync(); | ||
| // Mutual-exclusivity zeroing held: only insertBefore is enabled. | ||
| expect(props.choice.insertBefore?.enabled).toBe(true); | ||
| expect(props.choice.insertAfter.enabled).toBe(false); | ||
| expect(props.choice.prepend).toBe(false); | ||
| }); | ||
| }); |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.