Skip to content

Fix calendar popup ignoring reminder-format settings and migrate the hotkey chooser to CM6#337

Merged
uphy merged 3 commits into
masterfrom
worktree-fix-hotkey-datetime-chooser-cm6
Jul 12, 2026
Merged

Fix calendar popup ignoring reminder-format settings and migrate the hotkey chooser to CM6#337
uphy merged 3 commits into
masterfrom
worktree-fix-hotkey-datetime-chooser-cm6

Conversation

@uphy

@uphy uphy commented Jul 12, 2026

Copy link
Copy Markdown
Owner

Closes #248

Root cause

Calendar.svelte constructed its own new Settings() just to read the week-start value. The Settings constructor ended with a global side effect: it installed a reminder-format config bound to that fresh, never-loaded instance via setReminderFormatConfig(). As a result, every time the date/time chooser was mounted, all reminder-format settings (e.g. useCustomEmojiForTasksPlugin, useReminderTimeFallbackForTasksPlugin) silently reverted to their defaults for the rest of the session — which is why reminders inserted from the calendar popup came out as 📅 2026-07-20 instead of ⏰ 2026-07-20 09:15 📅 2026-07-20 09:15. This affected the hotkey command, the (@ trigger, and the reminder-pill edit path alike.

The CM5-era popup code was a separate, secondary problem: the hotkey command still used the old DateTimeChooserView, which throws on CM6 (charCoords/defaultTextHeight) and silently fell back to the center modal.

Changes

  • Migrate the hotkey date/time chooser to the CM6 popup: AutoComplete.show() now uses the same CM6DateTimeChooserPopup as the (@ trigger (PR Show date/time chooser inline near the cursor in Live Preview #307) instead of the CM5-only DateTimeChooserView (deleted), removing the exception → modal fallback.
  • Stop the calendar popup from resetting reminder-format settings:
    • Calendar.svelte takes weekStart as a prop, passed down from all three chooser entry points.
    • The config-installation block is extracted out of the Settings constructor into an explicit wireReminderFormatConfig() called once by PluginData, so constructing a Settings instance no longer has process-wide side effects.
  • Fix the calendar starting one day late when a week start is set: the week-start offset was applied twice (the settings tab mutated the bundled moment's locale dow, and the calendar added the explicit weekStart on top of locale-relative .weekday()), so "Week start: Monday" rendered a Tuesday-first calendar. All calendar math now uses the absolute .day() plus the explicit weekStart, and the moment.updateLocale side effect is removed. Weekend highlighting is an absolute Sat/Sun check, unchanged for the default locale. Covered by new unit tests in src/ui/calendar.test.ts.

Verification

  • mise run pre-commit: lint/typecheck/svelte-check clean; all tests pass except the pre-existing date-boundary flaky in src/model/reminder.test.ts (unrelated).
  • New unit tests for the Monday-start calendar grid and the weekend check.
  • Manually verified in a real vault (clean Obsidian start, hot-reload disabled): with useCustomEmojiForTasksPlugin + useReminderTimeFallbackForTasksPlugin enabled and Tasks as the primary format, both the hotkey command and the (@ trigger now open the inline popup near the cursor and insert ⏰ <date> <time> 📅 <date> <time> as configured. Before the fix, both inserted 📅 <date> only.

🤖 Generated with Claude Code

@uphy uphy added the bug Something isn't working label Jul 12, 2026
uphy and others added 2 commits July 13, 2026 00:31
show-date-chooser (used by the "Show date chooser" command/hotkey)
still built the legacy CM5 DateTimeChooserView, whose charCoords()/
defaultTextHeight() calls don't exist on Obsidian's current CM6
EditorView. In Live Preview this threw on every invocation and was
silently swallowed, falling back to the plain Modal chooser instead
of the intended inline popup — the same CM5-incompatibility already
fixed for the inline "(@" trigger in cm6-datetime-chooser.ts (added
alongside editor-extension.ts). Reuse that same CM6DateTimeChooserPopup
for the hotkey path and remove the now-unreferenced CM5 view.

Fixes #248
Calendar.svelte constructed its own `new Settings()` just to read the
week-start value. The Settings constructor ended with a global side
effect — it installed a config bound to that fresh, never-loaded
instance via `setReminderFormatConfig()` — so every time the date/time
chooser was mounted, all reminder-format settings (e.g.
`useCustomEmojiForTasksPlugin`) silently reverted to their defaults for
the rest of the session. This, not the CM5-era popup code, was the root
cause of the "settings are not reflected" half of #248, and it affected
the `(@` trigger and reminder-pill paths as well.

- Calendar.svelte now takes `weekStart` as a prop, passed down from all
  three chooser entry points (hotkey command, `(@` trigger, reminder
  pill). This also makes the calendar honor the user's actual week-start
  setting, which the stray default-valued instance never did.
- The config-installation block is extracted out of the Settings
  constructor into an explicit `wireReminderFormatConfig()` called once
  by PluginData, so constructing a Settings instance no longer has
  process-wide side effects.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@uphy
uphy force-pushed the worktree-fix-hotkey-datetime-chooser-cm6 branch from 52f24bd to b58a7b1 Compare July 12, 2026 15:32
The week-start offset was applied twice: the settings tab mutated the
bundled moment's locale (`moment.updateLocale` with the configured
`dow`), and the calendar then added the explicit `weekStart` offset on
top of the locale-relative `.weekday()`. With "Week start: Monday" the
calendar therefore rendered starting on Tuesday. The double application
had been masked while the calendar's weekStart was always the stray
default (0).

All calendar math now uses the absolute, locale-independent `.day()`
(0=Sunday) together with the explicit `weekStart` value, and the
`moment.updateLocale` side effect is removed — nothing depends on the
locale's `dow` anymore, and it only ever ran while the settings tab was
open, making behavior state-dependent. Weekend highlighting is now an
absolute Sat/Sun check, unchanged for the default locale.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@uphy
uphy merged commit c3bff2e into master Jul 12, 2026
2 checks passed
@uphy
uphy deleted the worktree-fix-hotkey-datetime-chooser-cm6 branch July 12, 2026 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Plugin is not saving the settings properly - returns to default settings partially

1 participant