Fix add-feed Save button doing nothing

Save mixed ButtonComponent.setDisabled() with raw toggleAttribute() and required
a separate Resolve click, so it could stay disabled/unresponsive. Save is now
always enabled and resolves the feed on demand before saving, with a re-entry
guard. Added a regression test for the resolve-on-save path.
This commit is contained in:
Charles Kelsoe 2026-06-14 16:16:47 -04:00
parent 9f083443da
commit 26b4debd2e
2 changed files with 108 additions and 15 deletions

View file

@ -0,0 +1,86 @@
// Regression test for the add-feed Save button. The button previously mixed
// Obsidian's ButtonComponent.setDisabled() with raw toggleAttribute() and stayed
// disabled, and required a prior Resolve click, so Save appeared to do nothing.
// Save now resolves on demand and calls onSave. No DOM is built here (onOpen is
// bypassed); the input fields are injected so save() can run headless.
import { App } from "obsidian";
import { AddFeedModal, type AddFeedModalDeps } from "../add-feed-modal";
import { DEFAULT_SETTINGS, type FeedConfig } from "../settings";
import type { FeedSource, ResolvedFeed } from "../feed-source";
function cannedResolved(): ResolvedFeed {
return {
sourceType: "generic",
feedId: "example.com",
canonicalHost: "example.com",
feedUrl: "https://example.com/feed",
publicationTitle: "Example",
author: "Anne Author",
sampleTitles: ["One", "Two"],
audienceHint: "unknown",
};
}
interface ModalInternals {
inputEl: { value: string };
folderInputEl: { value: string };
tagsInputEl: { value: string };
save(): Promise<void>;
}
function makeModal(
onSave: (feed: FeedConfig) => Promise<void>,
resolve: () => Promise<ResolvedFeed> = () => Promise.resolve(cannedResolved()),
): AddFeedModal {
const source: FeedSource = {
type: "generic",
resolve,
listItems: () => Promise.resolve([]),
fetchBody: (item) => Promise.resolve(item),
};
const deps: AddFeedModalDeps = {
settings: { ...DEFAULT_SETTINGS },
makeSource: () => ({ source }),
onSave,
};
const modal = new AddFeedModal(new App(), deps);
const internals = modal as unknown as ModalInternals;
internals.inputEl = { value: "https://example.com/feed" };
internals.folderInputEl = { value: "" };
internals.tagsInputEl = { value: "reading, feeds" };
return modal;
}
describe("AddFeedModal save", () => {
it("resolves on demand and saves when Save is clicked without a prior Resolve", async () => {
const saved: FeedConfig[] = [];
const modal = makeModal((feed) => {
saved.push(feed);
return Promise.resolve();
});
await (modal as unknown as ModalInternals).save();
expect(saved).toHaveLength(1);
const feed = saved[0];
expect(feed?.feedUrl).toBe("https://example.com/feed");
expect(feed?.sourceType).toBe("generic");
expect(feed?.canonicalHost).toBe("example.com");
// Empty folder input falls back to <parent>/<publication>.
expect(feed?.destinationFolder).toBe("Feeds/Example");
expect(feed?.tags).toEqual(["reading", "feeds"]);
});
it("does not call onSave when resolve fails", async () => {
let calls = 0;
const modal = makeModal(
() => {
calls += 1;
return Promise.resolve();
},
() => Promise.reject(new Error("network down")),
);
await (modal as unknown as ModalInternals).save();
expect(calls).toBe(0);
});
});

View file

@ -69,8 +69,8 @@ export class AddFeedModal extends Modal {
private folderInputEl: HTMLInputElement | null = null;
private tagsInputEl: HTMLInputElement | null = null;
private previewEl: HTMLDivElement | null = null;
private saveButtonEl: HTMLButtonElement | null = null;
private folderEdited = false;
private saving = false;
private focusTimer: number | null = null;
constructor(app: App, deps: AddFeedModalDeps) {
@ -151,16 +151,14 @@ export class AddFeedModal extends Modal {
this.close();
}),
);
footer.addButton((btn) => {
footer.addButton((btn) =>
btn
.setButtonText("Save")
.setCta()
.setDisabled(true)
.onClick(() => {
void this.save();
});
this.saveButtonEl = btn.buttonEl;
});
}),
);
this.focusTimer = window.setTimeout(() => {
this.focusTimer = null;
@ -185,7 +183,6 @@ export class AddFeedModal extends Modal {
new Notice("Enter a feed URL or handle first.");
return;
}
this.setSaveDisabled(true);
this.renderPreviewLoading();
try {
const { source } = this.deps.makeSource(input);
@ -194,7 +191,6 @@ export class AddFeedModal extends Modal {
this.resolved = resolved;
this.renderPreviewCard(resolved);
this.seedFolderDefault(resolved);
this.setSaveDisabled(false);
} catch (err) {
this.resolved = null;
this.source = null;
@ -208,8 +204,25 @@ export class AddFeedModal extends Modal {
// then hand it to the caller. Guards against a Save click that races the
// resolve state.
private async save(): Promise<void> {
if (this.saving) {
return;
}
this.saving = true;
try {
await this.saveResolvedFeed();
} finally {
this.saving = false;
}
}
private async saveResolvedFeed(): Promise<void> {
// Resolve on demand when Save is clicked without a prior Resolve, so Save
// works whether or not the user used the Resolve button first.
if (this.resolved === null) {
new Notice("Resolve a feed before saving.");
await this.resolveInput();
}
if (this.resolved === null) {
// resolveInput already surfaced the failure; nothing to save.
return;
}
const resolved = this.resolved;
@ -256,12 +269,6 @@ export class AddFeedModal extends Modal {
);
}
private setSaveDisabled(disabled: boolean): void {
if (this.saveButtonEl !== null) {
this.saveButtonEl.toggleAttribute("disabled", disabled);
}
}
private renderPreviewPlaceholder(): void {
const el = this.previewEl;
if (el === null) {