Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a date-picker UI and integrations across prompts and modal inputs, updates formatter to treat Changes
Sequence DiagramsequenceDiagram
participant User
participant QuickAddAPI
participant VDateInputPrompt
participant DatePicker
participant Formatter
User->>QuickAddAPI: datePrompt(header, options)
QuickAddAPI->>VDateInputPrompt: show prompt
VDateInputPrompt->>DatePicker: createDatePicker(container)
DatePicker-->>VDateInputPrompt: ready
alt User selects date in picker
User->>DatePicker: click date
DatePicker->>VDateInputPrompt: onSelect(iso,"picker")
VDateInputPrompt->>VDateInputPrompt: syncPickerSelection(iso)
VDateInputPrompt->>VDateInputPrompt: renderPreviewFromIso(iso)
else User types input
User->>VDateInputPrompt: type input
VDateInputPrompt->>VDateInputPrompt: renderPreviewFromInput(value)
VDateInputPrompt->>DatePicker: setSelectedIso(iso?)
end
User->>VDateInputPrompt: submit
VDateInputPrompt->>VDateInputPrompt: transformInputOnSubmit(input) -> value
VDateInputPrompt->>Formatter: store value
Formatter->>Formatter: if startsWith("@date:") skip parse else parse/normalize (or error)
VDateInputPrompt-->>QuickAddAPI: return value / cancel
QuickAddAPI-->>User: promise resolved/rejected
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/preflight/OnePageInputModal.ts (1)
193-200: Missing cleanup for date picker controller.The
datePickeris created viacreateDatePicker()but there's no correspondingdestroy()call when the modal closes. While Obsidian modals typically clean up their DOM, the date picker may hold references or event listeners that should be explicitly released.Consider storing the controller and calling
destroy()in the modal's close handler.🔎 Suggested approach
Store date picker references and clean them up:
// Add field to class private datePickerControllers: DatePickerController[] = []; // In renderField for date type: this.datePickerControllers.push(datePicker); // Override onClose: onClose() { this.datePickerControllers.forEach(dp => dp.destroy()); super.onClose(); }src/gui/VDateInputPrompt/VDateInputPrompt.ts (1)
19-21: Missing cleanup for date picker controller.Similar to
OnePageInputModal, thedatePickercontroller is created butdestroy()is never called. Add cleanup inonClose():🔎 Suggested fix
onClose() { // Prevent any pending debounced updates this.isOpen = false; // Cancel any pending debounced calls this.updatePreviewDebounced.cancel(); + // Clean up date picker + this.datePicker?.destroy(); + // If input is empty and we have a default, use the default if (!this.input.trim() && this.defaultValue) { this.input = this.defaultValue; } super.onClose(); }src/gui/date-picker/datePicker.ts (3)
176-181: Consider creating the ARIA formatter once.The
ariaFormatteris recreated on every render, which is called whenever the selection or view month changes. Since the formatter options are static, creating it once outside the render function would improve efficiency.🔎 Suggested refactor
Move the formatter creation to line 75 (after
weekdayLabels):const weekStartsOn = getWeekStartsOn(options.weekStartsOn); const weekdayLabels = getWeekdayLabels(weekStartsOn); + const ariaFormatter = new Intl.DateTimeFormat(undefined, { + weekday: "long", + year: "numeric", + month: "long", + day: "numeric", + }); const initialDate = parseIsoToDate(options.initialIso) ?? new Date();Then remove lines 176-181 from the render function.
183-214: Consider simplifying date calculation in the render loop.Lines 184-185 compute each cell's date by creating a copy of
startDateand then setting the date tostartDate.getDate() + i. While correct, this pattern is less idiomatic and slightly inefficient.🔎 More idiomatic alternative
for (let i = 0; i < 42; i += 1) { - const date = new Date(startDate); - date.setDate(startDate.getDate() + i); + const date = new Date(startDate.getTime() + i * 86400000); const dayKey = toDateKey(date);Or use
setDateon the copied date itself:for (let i = 0; i < 42; i += 1) { const date = new Date(startDate); - date.setDate(startDate.getDate() + i); + date.setDate(date.getDate() + i); const dayKey = toDateKey(date);
85-122: Good accessibility implementation.The component includes proper ARIA labels for navigation buttons (lines 91, 100) and day buttons (line 197), sets
aria-pressedfor selected days (line 203), and correctly specifiestype="button"to prevent form submission.Optional enhancement: Consider adding
role="grid"to the calendar grid and anaria-liveregion for month changes to improve screen reader announcements during navigation.Also applies to: 192-213
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/formatters/formatter.ts(1 hunks)src/formatters/vdate-default.test.ts(1 hunks)src/gui/GenericInputPrompt/GenericInputPrompt.ts(2 hunks)src/gui/VDateInputPrompt/VDateInputPrompt.ts(7 hunks)src/gui/date-picker/datePicker.ts(1 hunks)src/preflight/OnePageInputModal.ts(3 hunks)src/quickAddApi.ts(3 hunks)src/styles.css(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Source code lives in
src/: core logic underengine/,services/, andutils/; Svelte UI insrc/gui; shared types insrc/types; settings entry insrc/quickAddSettingsTab.ts
Files:
src/preflight/OnePageInputModal.tssrc/formatters/vdate-default.test.tssrc/gui/GenericInputPrompt/GenericInputPrompt.tssrc/gui/VDateInputPrompt/VDateInputPrompt.tssrc/formatters/formatter.tssrc/gui/date-picker/datePicker.tssrc/quickAddApi.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Biome enforces tab indentation (width 2), LF endings, and an 80-character line guide; align editor settings
Use camelCase for variables and functions
Prefer type-only imports in TypeScript files
Route logging through theloggerutilities for consistent output
Structure production code so Obsidian dependencies are injected behind interfaces; unit tests target pure logic and swap in adapters ortests/obsidian-stub.ts
Files:
src/preflight/OnePageInputModal.tssrc/formatters/vdate-default.test.tssrc/gui/GenericInputPrompt/GenericInputPrompt.tssrc/gui/VDateInputPrompt/VDateInputPrompt.tssrc/formatters/formatter.tssrc/gui/date-picker/datePicker.tssrc/quickAddApi.ts
src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components
Files:
src/preflight/OnePageInputModal.tssrc/formatters/vdate-default.test.tssrc/gui/GenericInputPrompt/GenericInputPrompt.tssrc/gui/VDateInputPrompt/VDateInputPrompt.tssrc/formatters/formatter.tssrc/gui/date-picker/datePicker.tssrc/quickAddApi.ts
🧠 Learnings (1)
📚 Learning: 2025-12-09T21:20:52.425Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.425Z
Learning: Applies to tests/**/*.{ts,tsx} : Add regression coverage for bug fixes
Applied to files:
src/formatters/vdate-default.test.ts
🧬 Code graph analysis (6)
src/preflight/OnePageInputModal.ts (3)
src/utils/dateParser.ts (2)
formatISODate(70-85)parseNaturalLanguageDate(21-62)src/gui/VDateInputPrompt/VDateInputPrompt.ts (4)
createDatePicker(99-112)applyPickerSelection(220-231)clearPickerSelection(233-243)updatePreview(165-208)src/gui/date-picker/datePicker.ts (1)
createDatePicker(71-243)
src/gui/GenericInputPrompt/GenericInputPrompt.ts (1)
tests/obsidian-stub.ts (1)
TextComponent(122-149)
src/gui/VDateInputPrompt/VDateInputPrompt.ts (2)
src/gui/date-picker/datePicker.ts (2)
DatePickerController(3-10)createDatePicker(71-243)src/utils/dateParser.ts (2)
formatISODate(70-85)parseNaturalLanguageDate(21-62)
src/formatters/formatter.ts (2)
src/settingsStore.ts (1)
settingsStore(8-21)src/utils/dateAliases.ts (1)
normalizeDateInput(15-30)
src/gui/date-picker/datePicker.ts (2)
tests/obsidian-stub.ts (1)
moment(475-475)src/gui/VDateInputPrompt/VDateInputPrompt.ts (1)
createDatePicker(99-112)
src/quickAddApi.ts (2)
src/gui/VDateInputPrompt/VDateInputPrompt.ts (1)
VDateInputPrompt(12-324)src/utils/dateParser.ts (1)
formatISODate(70-85)
🪛 GitHub Actions: Test
src/gui/VDateInputPrompt/VDateInputPrompt.ts
[error] 12-12: TypeError: Class extends value # is not a constructor or null. The class VDateInputPrompt extends GenericInputPrompt likely due to an incorrect import or circular dependency.
src/quickAddApi.ts
[error] 19-19: TypeError: Class extends value # is not a constructor or null. Retrieved during test execution; stack may reference VDateInputPrompt.ts.
🔇 Additional comments (13)
src/gui/GenericInputPrompt/GenericInputPrompt.ts (2)
13-13: LGTM - Visibility change enables subclass access.Changing
inputComponentfromprivatetoprotectedallows subclasses likeVDateInputPromptto access the input element for synchronization with the date picker.
177-183: LGTM - Clean hook pattern for input transformation.The
transformInputOnSubmithook with an identity default enables subclasses to customize submission behavior without modifying the base class logic. The submit flow correctly reads the raw value, transforms it, and assigns the result.src/formatters/vdate-default.test.ts (1)
192-200: LGTM - Good regression test for @Date: bypass behavior.This test verifies the critical behavior that values prefixed with
@date:bypass date parsing and are formatted directly using the stored ISO string. This aligns with the formatter changes and provides regression coverage. Based on learnings, this adds appropriate coverage for the new feature.src/styles.css (1)
174-271: LGTM - Well-structured date picker styles.The new date picker CSS uses appropriate CSS custom properties for theme consistency (e.g.,
var(--background-modifier-border),var(--interactive-accent)). The grid layout, hover states, and action button styling are clean and follow existing QuickAdd patterns.src/formatters/formatter.ts (1)
475-499: LGTM - Robust date handling with proper guards.The logic correctly:
- Bypasses parsing for values already prefixed with
@date:- Guards against missing
dateParserbefore attempting normalization/parsing- Normalizes input using aliases before parsing
- Throws explicit errors when parsing fails, preventing silent failures
This ensures consistent
@date:ISOstorage and proper error propagation.src/preflight/OnePageInputModal.ts (1)
263-283: LGTM - Proper default value handling with picker sync.The
updatePreviewfunction correctly handles the default value case by parsing it, storing the ISO string, syncing the picker selection, and rendering the preview. The error path also properly clears state and shows the error message.src/quickAddApi.ts (1)
555-584: LGTM - Clean datePrompt API implementation.The implementation correctly:
- Delegates to
VDateInputPrompt.Promptwith the provided options- Extracts the ISO string from
@date:prefixed values- Applies optional
dateFormatfor user-friendly output- Returns the raw ISO if no format is specified or formatting fails
- Follows existing error handling patterns with
throwIfPromptCancelledsrc/gui/VDateInputPrompt/VDateInputPrompt.ts (3)
99-112: LGTM - Date picker integration is well-structured.The
createDatePickermethod properly:
- Creates a container with appropriate styling class
- Initializes the picker with the current
selectedIso- Wires up
onSelectto apply or clear selectionsThe picker controller is stored for later synchronization.
282-308: LGTM - Robust input transformation on submit.The
transformInputOnSubmitmethod correctly handles all cases:
- Already prefixed with
@date:→ return as-is- Input matches picker selection → convert to
@date:ISO- Empty with default value → parse default and convert
- Regular input → parse and convert if valid
- Fallback → return original input
This ensures consistent
@date:prefixed output for downstream processing.
3-12: Import structure is correct—no circular dependency exists.Investigation confirms the import chain is one-directional:
VDateInputPromptimportsGenericInputPrompt, which does not import back fromVDateInputPromptor any module that would create a cycle.GenericInputPromptproperly exportsexport default class. The originally reported pipeline failure, if real, is not caused by the circular dependency concern described in this comment.Likely an incorrect or invalid review comment.
src/gui/date-picker/datePicker.ts (3)
1-17: LGTM! Well-structured public API.The type definitions and interfaces provide a clean, well-typed API surface with clear semantics for selection sources and controller methods.
220-230: LGTM! Correct default behavior.The
updateView !== falsecheck ensures the view updates by default (whenupdateViewis undefined), while allowing explicit opt-out withupdateView: false. This provides a good developer experience.
21-28: LGTM! Robust fallback logic.The function properly validates the explicit
weekStartsOnoption, falls back to the moment locale setting if available, and defaults to 0 (Sunday). The range checks and optional chaining ensure safe operation.
| const toIsoFromParts = (year: number, month: number, day: number): string => { | ||
| const moment = window.moment; | ||
| if (moment) { | ||
| return moment({ year, month, day }).startOf("day").toISOString(); | ||
| } | ||
| return new Date(year, month, day).toISOString(); | ||
| }; |
There was a problem hiding this comment.
Consider using UTC midnight for consistent date handling.
The current implementation creates dates at midnight local time, then converts to ISO UTC. This causes the ISO string's date portion to shift for timezones ahead of UTC.
Example: User in UTC+8 selects January 1, 2024:
- Line 68 produces:
"2023-12-31T16:00:00.000Z" - Date portion shows December 31, not January 1
This can lead to confusion if consumers parse the ISO string as a UTC date or if data crosses timezones.
🔎 Proposed fix for consistent UTC midnight
const toIsoFromParts = (year: number, month: number, day: number): string => {
const moment = window.moment;
if (moment) {
- return moment({ year, month, day }).startOf("day").toISOString();
+ return moment.utc({ year, month, day }).toISOString();
}
- return new Date(year, month, day).toISOString();
+ return new Date(Date.UTC(year, month, day)).toISOString();
};This ensures the ISO date portion always matches the selected calendar date.
Note: Line 66's .startOf("day") is redundant since moment({ year, month, day }) already creates midnight.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const toIsoFromParts = (year: number, month: number, day: number): string => { | |
| const moment = window.moment; | |
| if (moment) { | |
| return moment({ year, month, day }).startOf("day").toISOString(); | |
| } | |
| return new Date(year, month, day).toISOString(); | |
| }; | |
| const toIsoFromParts = (year: number, month: number, day: number): string => { | |
| const moment = window.moment; | |
| if (moment) { | |
| return moment.utc({ year, month, day }).toISOString(); | |
| } | |
| return new Date(Date.UTC(year, month, day)).toISOString(); | |
| }; |
🤖 Prompt for AI Agents
In src/gui/date-picker/datePicker.ts around lines 63 to 69, the function creates
midnight in local time then converts to ISO which shifts the date for non-UTC
timezones; change it to create UTC-midnight instead: when moment is available
use moment.utc({ year, month, day }) (no startOf needed) and otherwise use new
Date(Date.UTC(year, month, day)).toISOString() so the ISO date portion always
matches the selected calendar date.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/gui/date-picker/datePicker.ts (1)
76-78: Consider renaming to reflect date-only format.The function name
toIsoFromPartssuggests a full ISO 8601 timestamp, but it returns only the date portion ("YYYY-MM-DD"). Consider renaming totoDateStringFromPartsorformatDateFromPartsfor clarity.🔎 Proposed rename
-const toIsoFromParts = (year: number, month: number, day: number): string => { +const toDateStringFromParts = (year: number, month: number, day: number): string => { return formatDateKey(year, month, day); };Then update the call sites at lines 161, 216.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/gui/date-picker/datePicker.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Source code lives in
src/: core logic underengine/,services/, andutils/; Svelte UI insrc/gui; shared types insrc/types; settings entry insrc/quickAddSettingsTab.ts
Files:
src/gui/date-picker/datePicker.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Biome enforces tab indentation (width 2), LF endings, and an 80-character line guide; align editor settings
Use camelCase for variables and functions
Prefer type-only imports in TypeScript files
Route logging through theloggerutilities for consistent output
Structure production code so Obsidian dependencies are injected behind interfaces; unit tests target pure logic and swap in adapters ortests/obsidian-stub.ts
Files:
src/gui/date-picker/datePicker.ts
src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components
Files:
src/gui/date-picker/datePicker.ts
🧬 Code graph analysis (1)
src/gui/date-picker/datePicker.ts (2)
tests/obsidian-stub.ts (1)
moment(475-475)src/gui/VDateInputPrompt/VDateInputPrompt.ts (1)
createDatePicker(99-112)
🔇 Additional comments (3)
src/gui/date-picker/datePicker.ts (3)
30-44: Timezone bug fixed—weekday labels now render correctly.The previous UTC timestamp issue has been resolved. The implementation now creates dates in local time and uses date arithmetic to avoid timezone shifts.
76-78: Approach changed: now returns date-only string.The previous concern about UTC midnight has been addressed by switching to a simpler approach—returning
"YYYY-MM-DD"format instead of a full ISO 8601 string. This avoids timezone complexity entirely.
80-252: Implementation looks solid.The date picker logic is well-structured:
- Proper state management for view and selection
- DST-safe date arithmetic in the rendering loop (lines 193-194)
- Accessible UI with ARIA labels and button types
- Clean public API with controller pattern
- Boundary conversion from internal
undefinedto externalnullfor cleared selections (lines 150-152) maintains API consistency
|
🎉 This PR is included in version 2.10.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Summary
datePrompthelper and date picker stylesReproduction
{{VDATE:due,YYYY-MM-DD}}).quickAddApi.requestInputs).Testing
bun run build-with-lintSummary by CodeRabbit
New Features
Bug Fixes / Behavior
Tests
Style
✏️ Tip: You can customize this high-level summary in your review settings.