Skip to content

fix(settings): keep focused text through synced refreshes - #500

Merged
chhoumann merged 5 commits into
masterfrom
fix/settings-focused-sync
Oct 15, 2026
Merged

chhoumann merged 5 commits into
masterfrom
fix/settings-focused-sync

Conversation

@chhoumann

@chhoumann chhoumann commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

Summary

Keep the text and selection in a focused settings field when sync arrives. Memory adopts the synced value immediately; blur shows it without saving the old text again. Further typing still saves normally.

Obsidian rebuilds custom settings rows after its external-change callback. Capture the focused row before cleanup and consume that snapshot once, so it cannot return when Settings reopens. Keep the change inside settings UI; persistence and adoption wiring are unchanged. Deferring refresh until blur lost toggle clicks in real Obsidian.

Fixes #496

Verification

  • Linux, Node 22.23.2, isolated Obsidian 1.14.4 and 1.13.7. Synthetic data.json sync writes; no feed or audio required. Red regression committed before the fix; direct callback plus host refresh and natural watcher paths tested.
  • Final target test passed 20 consecutive runs. Complete settings file passed 5 consecutive runs, plus all 11 settings tests on 1.13.7. Tests cover backward selection, repeated sync, clicks, blur, continued typing, numeric inputs and reopening.
  • npm run lint, npm run format:check, npm run typecheck, npm run build, npm run test (2,769 tests and zero Svelte errors/warnings), npm run test:tooling (35 tests), npm run docs:build (18 pages, 112 legacy fragments) passed.
  • Fresh-vault npm run test:e2e passed on 1.14.4 before the base merge (232 tests). PODNOTES_OBSIDIAN_APP=/opt/Obsidian/obsidian npm run test:e2e passed on the merged head with 1.13.7 (236 tests). The first floor run had one popout-player failure from Obsidian's getZoomFactor error. The unchanged full retry passed; both logs are included.
  • Six mutations fail the settings regressions. Input-only preservation, missing completion redraw, missing selection, missing selection direction, and reapplying preserved text as input fail a settings refresh keeps the focused %s text and selection until completion. Selection restoration on number inputs fails a refresh keeps a focused number without trying to restore a text selection. Retaining a snapshot after its first render also failed the native sync tests at reopen.
  • After merging origin/master 39805c53, main.js is 103,025 gzip bytes, 25 over the unchanged 103,000 budget. Base is 102,916, so this fix adds 109 gzip bytes. Raw size is 321,175 bytes, +292. npm run size currently fails; the coordinator will land SIZE2, update this branch and recheck size before merging. A snapshot around update() was larger at 103,028 gzip and restored text after preview rendering. Logs, mutations, size comparison and review.

Before | After, still focused | After blur

Release impact

No storage, API or URI changes and no migration. Obsidian still rebuilds the controls; existing IME/undo behavior is unchanged.

Docs at release

None.


Note

Medium Risk
Touches settings UI draft/completion timing during external sync; behavior is well-covered by tests but incorrect edge cases could affect what gets saved or displayed on blur.

Overview
Fixes settings fields losing in-progress edits when synced settings trigger a tab refresh (fixes #496).

When Obsidian rebuilds settings rows after external sync, textRow now snapshots the focused control’s value, selection, and direction (matched by name), reapplies that draft on the next render, then clears the snapshot so it cannot leak on reopen. Number fields keep the draft value but skip selection restore. bindDraft’s completion path always runs show() so blur/completion surfaces the synced in-memory value without re-saving stale text.

Unit and e2e tests cover refresh during focus, repeated sync, completion via blur/click, continued typing, and numeric inputs. The Obsidian settings-tab mock fetches definitions before row cleanup so the snapshot can read the active element.

Reviewed by Cursor Bugbot for commit eca578f. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Fix settings tab losing focused text during synced refreshes

  • Focus on a settings text field is no longer lost when the tab re-renders after a settings sync. PodNotesSettingsTab.ts now snapshots the focused control's value, selection bounds, and direction when a text row is recreated, and restores the captured value and selection after the refresh
  • Draft completion in PodNotesSettingsTab.bindDraft now runs the control-restoration path even when the draft is no longer registered, while duplicate completion callbacks still skip listener removal and pending saves
  • Number controls keep their draft value but do not get a selection-range restore, since they have no text selection
  • Adds unit tests in PodNotesSettingsTab.test.ts and an end-to-end test in settings-tab.test.ts covering focused refresh, completion, and post-refresh editing
  • Updates the Obsidian mock in obsidian.ts so setting definitions are captured before old rows are cleared, letting definition construction inspect the active control

Macroscope summarized eca578f.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Deploying podnotes with  Cloudflare Pages  Cloudflare Pages

Latest commit: eca578f
Status: ✅  Deploy successful!
Preview URL: https://e884061a.podnotes.pages.dev
Branch Preview URL: https://fix-settings-focused-sync.podnotes.pages.dev

View logs

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit eca578f. Configure here.

start: active.selectionStart,
end: active.selectionEnd,
direction: active.selectionDirection,
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Number fields crash on refresh snapshot

Medium Severity

A focused number row snapshots selectionStart, selectionEnd, and selectionDirection during rebuild. Chromium throws InvalidStateError for those getters on type="number", so a sync refresh while Skip back/forward, Timestamp offset, or Latest episodes is focused aborts getSettingDefinitions and the tab never shows the adopted values. Restore already skips a null start; the capture does not.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit eca578f. Configure here.

@chhoumann
chhoumann merged commit 791912f into master Oct 15, 2026
18 checks passed
@chhoumann
chhoumann deleted the fix/settings-focused-sync branch October 15, 2026 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

E2E: settings blur test sometimes sees a synced value replace the focused field

1 participant