feat(core, prompts): add DatePrompt for date input with customizable formats #448
Conversation
🦋 Changeset detectedLatest commit: 24c385a The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
natemoo-re
left a comment
natemoo-re
left a comment
There was a problem hiding this comment.
Looking great so far! Left a few comments—happy to clarify if anything doesn't make sense.
| "@clack/prompts": minor | ||
| "@clack/core": minor |
There was a problem hiding this comment.
Because we're v1.0, we're going to have to hold minor features until we're ready to do a release! Does a 2 week cadence seem fair?
Will spin up a discussion in Discord
There was a problem hiding this comment.
let's discuss on discord 👍
i think that's fine though
| const MONTH_NAMES = [ | ||
| 'January', | ||
| 'February', | ||
| 'March', | ||
| 'April', | ||
| 'May', | ||
| 'June', | ||
| 'July', | ||
| 'August', | ||
| 'September', | ||
| 'October', | ||
| 'November', | ||
| 'December', | ||
| ]; |
There was a problem hiding this comment.
Think these should live in the global settings so they can be localized
| function toISOString(display: string, config: FormatConfig): string | undefined { | ||
| const { year, month, day } = parseDisplayString(display, config); | ||
| if (!year || year < 1000 || year > 9999) return undefined; | ||
| if (!month || month < 1 || month > 12) return undefined; | ||
| if (!day || day < 1) return undefined; | ||
| const date = new Date(year, month - 1, day); | ||
| if (date.getFullYear() !== year || date.getMonth() !== month - 1 || date.getDate() !== day) { | ||
| return undefined; | ||
| } | ||
| return `${year}-${String(month).padStart(2, '0')}-${String(day).padStart(2, '0')}`; | ||
| } |
There was a problem hiding this comment.
Is there a reason we can't use Date().toISOString()?
| const { year, month, day } = parseDisplayString( | ||
| this.userInput || this.#config.displayTemplate, | ||
| this.#config | ||
| ); |
There was a problem hiding this comment.
I notice we're parsing from the display string on every update... feels like we could remove some code by storing the segments on the class and having the display string use a getter?
| interface FormatConfig { | ||
| segments: SegmentConfig[]; | ||
| displayTemplate: string; | ||
| format: (year: string, month: string, day: string) => string; |
There was a problem hiding this comment.
Could we refactor this to be an object with { year, month, date }? Object callbacks are more refactor-proof if we want to expose additional options in the future.
General rule of thumb is that if you find yourself writing >=3 arguments, you should stick use an object instead.
| if (seg.type === 'month' && (month < 1 || month > 12)) { | ||
| return 'There are only 12 months in a year'; | ||
| } | ||
| if (seg.type === 'day') { | ||
| if (day < 1) return undefined; // incomplete | ||
| const daysInMonth = new Date(year || 2000, month || 1, 0).getDate(); | ||
| if (day > daysInMonth) { | ||
| const monthName = month >= 1 && month <= 12 ? MONTH_NAMES[month - 1] : 'this month'; | ||
| return `There are only ${daysInMonth} days in ${monthName}`; | ||
| } | ||
| } |
There was a problem hiding this comment.
More strings for localization 😉
| return undefined; | ||
| } | ||
|
|
||
| export interface DateOptions extends PromptOptions<string, DatePrompt> { |
There was a problem hiding this comment.
The Date selector should operate on a Date rather than a string? This prompt should return a Date value. Might simplify some of the internal logic as well.
| const ctx = this.#getSegmentAtCursor(); | ||
| if (!ctx) return; | ||
| const seg = ctx.segment; | ||
| const display = this.userInput || this.#config.displayTemplate; | ||
| const segmentDisplay = display.slice(seg.start, seg.start + seg.len); | ||
|
|
||
| // Inject at leftmost blank when filling, or at cursor when editing filled segment | ||
| // Guarantees left-to-right: "____" → "2___" → "20__" → "202_" → "2025" | ||
| const firstBlank = segmentDisplay.indexOf('_'); | ||
| const pos = firstBlank >= 0 ? firstBlank : Math.min(this._cursor - seg.start, seg.len - 1); | ||
| if (pos < 0 || pos >= seg.len) return; | ||
|
|
||
| const newSegmentVal = segmentDisplay.slice(0, pos) + char + segmentDisplay.slice(pos + 1); | ||
| const newDisplay = | ||
| display.slice(0, seg.start) + newSegmentVal + display.slice(seg.start + seg.len); | ||
|
|
||
| // Validate month (1-12) and day (1 to daysInMonth) when segment is full | ||
| if (!newSegmentVal.includes('_')) { | ||
| const validationMsg = getSegmentValidationMessage(newDisplay, seg, this.#config); | ||
| if (validationMsg) { | ||
| this.inlineError = validationMsg; | ||
| return; | ||
| } | ||
| } | ||
| this.inlineError = ''; | ||
|
|
||
| const iso = toISOString(newDisplay, this.#config); | ||
|
|
||
| if (iso) { | ||
| const { year, month, day } = parseDisplayString(newDisplay, this.#config); | ||
| const clamped = formatDisplayString( | ||
| { | ||
| year: clampSegment(year, 'year', { year, month }), | ||
| month: clampSegment(month, 'month', { year, month }), | ||
| day: clampSegment(day, 'day', { year, month }), | ||
| }, | ||
| this.#config | ||
| ); | ||
| this._setUserInput(clamped); | ||
| this._setValue(iso); | ||
| } else { | ||
| this._setUserInput(newDisplay); | ||
| this._setValue(undefined); | ||
| } | ||
|
|
||
| // Cursor: next blank when filling; when segment just became full (was filling), jump to next | ||
| const nextBlank = newSegmentVal.indexOf('_'); | ||
| const wasFilling = firstBlank >= 0; // had blanks before this keystroke | ||
| if (nextBlank >= 0) { | ||
| this._cursor = seg.start + nextBlank; | ||
| } else if (wasFilling && ctx.index < this.#config.segments.length - 1) { | ||
| // Just completed segment by filling - jump to next | ||
| this._cursor = this.#config.segments[ctx.index + 1].start; | ||
| } else { | ||
| // Editing full segment - advance within or stay at end | ||
| this._cursor = seg.start + Math.min(pos + 1, seg.len - 1); | ||
| } |
There was a problem hiding this comment.
This is doing a lot of imperative mutation—if the prompt stored a richer representation of its cursor state, we could avoid recalculating the location on keypress. Callbacks update state, display should be derived from the state.
| message: string; | ||
| format?: DateFormat; | ||
| defaultValue?: string | Date; | ||
| initialValue?: string | Date; |
There was a problem hiding this comment.
Would it simplify anything to only accept Date?
| } | ||
| } | ||
| }, | ||
| }).prompt() as Promise<string | symbol>; |
There was a problem hiding this comment.
Still think returning a Date would be a better API. Users will have to convert it anyway.
| }).prompt() as Promise<string | symbol>; | |
| }).prompt() as Promise<Date | symbol>; |
Implements a date prompt that enforces a structured date format and supports the most common variants:
YYYY/MM/DD,MM/DD/YYYY, andDD/MM/YYYY.related: #447