feat: add terminal calendar date picker widget#1636
Conversation
github-actions
Bot
added
area:widgets
|
Warning Review limit reached
More reviews will be available in 24 minutes and 15 seconds. Learn how PR review limits work. To continue reviewing without waiting, enable usage-based billing in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new ChangesDatePicker Widget
packages/core Cleanup
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
packages/charts/package.json (1)
3-63:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winResolve the manifest merge conflict before merge.
The conflict markers make
package.jsoninvalid JSON, so package installation/build tooling cannot parse@termuijs/charts. Also choose one entrypoint strategy; publishingmain/exportsto./src/index.tsconflicts with the existing dist-based package metadata.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/charts/package.json` around lines 3 - 63, Remove the Git merge conflict markers (<<<<<<, =======, >>>>>>) from the package.json file in the packages/charts directory. Keep the incoming version from upstream/main which includes the complete and correct package configuration with proper version number (0.1.6), build scripts (tsup, typecheck), dependency declarations, keywords, license, and engines specification, along with the dist-based entry points strategy for main, module, types, and exports fields. Discard the HEAD version that points directly to src/index.ts as it conflicts with the established build and publishing strategy.Source: Linters/SAST tools
packages/charts/src/index.ts (1)
1-57:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRemove the conflict markers and keep the real widget exports.
This file cannot compile with the conflict markers present. When resolving it, avoid shipping the HEAD-side placeholder
AreaChart/PieChart/Gaugeclasses because they replace the widget-backed chart behavior with constant-string stubs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/charts/src/index.ts` around lines 1 - 57, Remove the merge conflict markers from the index.ts file and keep only the upstream/main side. Delete the HEAD-side implementation that contains the placeholder AreaChart, PieChart, and Gauge class definitions that return constant string stubs (like 'AreaChart', 'PieChart', 'Gauge(${this.value}%)'), and instead retain the proper re-export statements that import AreaChart, PieChart, and Gauge from `@termuijs/widgets` along with their corresponding type exports (AreaChartOptions, PieSlice, PieChartOptions, and GaugeOptions). Remove all conflict markers including the <<<<<<< HEAD, =======, and >>>>>>> upstream/main lines.Source: Linters/SAST tools
packages/ui/src/CommandPalette.ts (1)
129-151:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winResolve both render conflicts and preserve the Unicode fallback.
The conflict markers make
CommandPalettefail to parse. For the row prefix, keep thecaps.unicode ? '❯ ' : '> 'branch rather than the raw❯string so non-Unicode terminals get a safe ASCII fallback. As per coding guidelines, “Usecaps.unicodefor non-ASCII characters with an ASCII fallback.”Proposed cleanup for the row-label conflict
-<<<<<<< HEAD - const label = - (active ? '❯ ' : ' ') + c.label; -======= const prefix = active ? (caps.unicode ? '❯ ' : '> ') : ' '; const shortcutStr = c.shortcut ? ` ${c.shortcut}` : ''; const labelFull = prefix + c.label + shortcutStr; const label = labelFull.slice(0, bw - 4).padEnd(bw - 4); ->>>>>>> upstream/main screen.writeString( bx + 1, by + 3 + rowOffset, -<<<<<<< HEAD - label.padEnd(bw - 4), -======= label, ->>>>>>> upstream/mainAlso applies to: 185-202
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/CommandPalette.ts` around lines 129 - 151, Remove the merge conflict markers in the grouped Map initialization and keep the upstream version that limits iteration to the vis slice (using this._maxVisible) rather than all this._filtered commands. Additionally, locate any instances where the Unicode character ❯ appears as a raw string for the row prefix and replace it with the conditional expression caps.unicode ? '❯ ' : '> ' to provide an ASCII fallback (>) for non-Unicode terminals. Apply the same conflict resolution and Unicode fallback fix to the second conflict section at lines 185-202 to ensure consistent handling throughout CommandPalette.Sources: Coding guidelines, Linters/SAST tools
packages/core/src/terminal/Screen.ts (1)
446-494:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winResolve the ANSI/SVG export conflict inside the class body.
The conflict markers split class methods and the SVG template literal, leaving
Screen.tssyntactically invalid.🐛 Proposed cleanup
-<<<<<<< HEAD - * Export current screen as ANSI snapshot text. - */ -exportANSI(): string { - const lines: string[] = []; - - for (let r = 0; r < this._rows; r++) { - lines.push(this.getLine(r)); - } - - return lines.join('\n'); -} - -/** - * Export current screen as SVG. - */ -exportSVG(): string { - return ` -======= * Export current screen as ANSI snapshot text. */ exportANSI(): string { const lines: string[] = []; for (let r = 0; r < this._rows; r++) { lines.push(this.getLine(r)); } return lines.join('\n'); } /** * Export current screen as SVG. */ exportSVG(): string { return ` ->>>>>>> upstream/main <svg xmlns="http://www.w3.org/2000/svg" width="${this._cols * 8}" height="${this._rows * 16}"> <text x="10" y="20"> Terminal Export </text> </svg>`; -<<<<<<< HEAD -} -======= } ->>>>>>> upstream/main🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/terminal/Screen.ts` around lines 446 - 494, The Screen.ts file has unresolved merge conflict markers (<<<<<<< HEAD, =======, >>>>>>> upstream/main) embedded within the exportANSI() and exportSVG() method definitions, making the file syntactically invalid. Resolve this conflict by accepting the upstream/main version which has the correct 4-space indentation for the class method bodies, then remove all conflict markers ensuring both exportANSI() and exportSVG() methods in the Screen class are complete and properly indented with consistent formatting.Source: Linters/SAST tools
packages/widgets/src/data/Table.ts (1)
139-171:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winKeep the
KeyEventhandler and clamp empty tables.The conflict markers make the class invalid. Resolve to the
handleKey(event: KeyEvent)contract; thekey: stringside breaks widget integration. Also guard the down-key path so an empty table cannot publishselectedRow === -1.As per coding guidelines,
packages/{widgets,ui}/src/**/*.{ts,tsx}: “A widget that handles keys takeshandleKey(event: KeyEvent)from@termuijs/core. Key names are lowercase.”🐛 Proposed cleanup
-<<<<<<< HEAD - handleKey(key: string): void { - if (key === 'up') { - this._selectedRow = Math.max(0, this._selectedRow - 1); - } - - if (key === 'down') { - this._selectedRow = Math.min( - this._rows.length - 1, - this._selectedRow + 1 - ); - } - - this.markDirty(); -} - -======= handleKey(event: KeyEvent): void { + const maxRow = Math.max(0, this._rows.length - 1); + if (event.key === 'up') { this._selectedRow = Math.max(0, this._selectedRow - 1); } if (event.key === 'down') { this._selectedRow = Math.min( - this._rows.length - 1, + maxRow, this._selectedRow + 1 ); } this.markDirty(); } - ->>>>>>> upstream/main🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widgets/src/data/Table.ts` around lines 139 - 171, Resolve the merge conflict markers in the handleKey method by keeping the upstream/main version that uses handleKey(event: KeyEvent) parameter, which is the correct widget integration contract. Additionally, add a guard condition in the down-key path to prevent selectedRow from becoming -1 when the table is empty; check if this._rows.length is greater than zero before allowing the selectedRow to be updated on the down key press to ensure empty tables cannot publish an invalid selectedRow state.Sources: Coding guidelines, Linters/SAST tools
packages/core/src/index.ts (1)
97-115:⚠️ Potential issue | 🔴 CriticalResolve the conflict markers and collapse the duplicate export block.
This barrel file cannot parse while lines 109–115 contain unresolved conflict markers. Lines 97–100 also duplicate the same utility/session exports (without
stripAnsiControl) that are correctly exported at lines 101–104, so resolve to one intended block before merge.🐛 Proposed cleanup
export { stringWidth, truncate, stripAnsi, wordWrap } from './utils/unicode.js'; export * as ansi from './utils/ansi.js'; -export { writeClipboard, readClipboard, clipboard } from './utils/ansi.js'; -export { debounce } from './utils/debounce.js'; -export type { DebounceOptions } from './utils/debounce.js'; -export * from './session/Session.js'; export { writeClipboard, readClipboard, clipboard, stripAnsiControl } from './utils/ansi.js'; export { debounce } from './utils/debounce.js'; export type { DebounceOptions } from './utils/debounce.js'; export * from './session/Session.js'; export { throttle } from './utils/throttle.js'; export type { ThrottleOptions } from './utils/throttle.js'; export { CommandHistory } from "./history/CommandHistory.js"; export type { CommandHistoryOptions } from "./history/CommandHistory.js"; -<<<<<<< HEAD -======= export * from './errors.js'; // ── Accessibility ───────────────────────────────────── export * from './a11y/index.js'; ->>>>>>> upstream/main🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/index.ts` around lines 97 - 115, Remove the unresolved git conflict markers (<<<<<<< HEAD, =======, >>>>>>> upstream/main) from the export block. The duplicate exports for writeClipboard, readClipboard, clipboard, debounce, DebounceOptions, and the Session module (appearing in both lines 97-100 and 101-104) should be consolidated into a single export statement that includes stripAnsiControl from the second occurrence. Keep all other unique exports including throttle, ThrottleOptions, CommandHistory, CommandHistoryOptions, errors.js exports, and a11y/index.js exports in their intended order.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/session/Session.ts`:
- Around line 35-50: The save() and restore() methods in the Session class
contain unresolved merge conflict markers with console.log statements on the
HEAD side that violate the no-console-log-in-source-files coding guideline and
leak sensitive session data. Remove all merge conflict markers (<<<<<<< HEAD,
=======, >>>>>>> upstream/main) from both the save() method and the restore()
method, and keep the empty implementations from the upstream/main side by
discarding the console.log calls entirely, leaving just the method signatures
with no body implementations.
In `@packages/store/src/index.ts`:
- Around line 9-12: The merge conflict markers in the export list of index.ts
are breaking module parsing. Remove the conflicted logger entry that appears to
be re-exported from ./store.js (the HEAD side of the conflict), keeping only the
logger export from ./logger.js which is already present on line 34. Delete the
entire conflict block including the HEAD markers and the logger line sourced
from ./store.js, allowing the upstream/main version to be used.
In `@packages/ui/src/index.ts`:
- Around line 32-45: The merge conflict markers at the export statements for
Shortcut and Notification types must be resolved by accepting the upstream/main
versions which use aliases. Remove all conflict markers (<<<<<<< HEAD, =======,
>>>>>>> upstream/main) and keep the aliased export versions: export Shortcut as
ShortcutManagerItem from ShortcutManager.js and export Notification as
NotificationHistoryItem from NotificationHistory.js. This prevents duplicate
type name exports since Shortcut is also exported from ShortcutHelpOverlay.js
and Notification is also exported from NotificationCenter.js, and allows
TypeScript to parse the file correctly.
In `@packages/widgets/src/display/Markdown.ts`:
- Around line 43-60: Resolve the merge conflict in the Markdown.ts file by
removing the conflict markers and accepting the upstream/main version. Change
the variable initialization from `segment = ''` to `segmentStr = ''` since the
downstream code (including the `flush()` method and the loop) already references
the `segmentStr` accumulator variable. Keep the cleaner single-line formatting
of the regex replace statement from the upstream version.
In `@packages/widgets/src/input/DatePicker.ts`:
- Around line 16-17: The format property is being stored in the DatePicker
constructor but is not actually used by the formatDate() method, which always
uses toLocaleDateString() instead. Update the formatDate() method in the
DatePicker class to respect and apply the this.format value when formatting
dates, so the format option provided through options becomes effective. The
method should use the stored format string to format dates appropriately rather
than ignoring it.
- Around line 13-16: The DatePicker constructor always initializes currentDate
to today's date using new Date(), regardless of whether a selectedDate option is
provided. When a selectedDate is provided in the options parameter, set
currentDate to the selectedDate value instead of always defaulting to today's
date. This ensures the calendar displays the correct month/year that corresponds
to the initial selection. Modify the assignment of this.currentDate in the
constructor to use options.selectedDate if it exists, otherwise fall back to new
Date().
- Around line 7-65: The DatePicker class must extend the Widget base class to
integrate with the widgets runtime. Change the class declaration to inherit from
Widget, then implement the required _renderSelf method that accepts a Screen
parameter to define how the DatePicker renders. Additionally, any method that
modifies visible state such as nextMonth, previousMonth, and selectDate should
call this.markDirty() after updating the state to notify the widget runtime that
re-rendering is needed.
---
Outside diff comments:
In `@packages/charts/package.json`:
- Around line 3-63: Remove the Git merge conflict markers (<<<<<<, =======,
>>>>>>) from the package.json file in the packages/charts directory. Keep the
incoming version from upstream/main which includes the complete and correct
package configuration with proper version number (0.1.6), build scripts (tsup,
typecheck), dependency declarations, keywords, license, and engines
specification, along with the dist-based entry points strategy for main, module,
types, and exports fields. Discard the HEAD version that points directly to
src/index.ts as it conflicts with the established build and publishing strategy.
In `@packages/charts/src/index.ts`:
- Around line 1-57: Remove the merge conflict markers from the index.ts file and
keep only the upstream/main side. Delete the HEAD-side implementation that
contains the placeholder AreaChart, PieChart, and Gauge class definitions that
return constant string stubs (like 'AreaChart', 'PieChart',
'Gauge(${this.value}%)'), and instead retain the proper re-export statements
that import AreaChart, PieChart, and Gauge from `@termuijs/widgets` along with
their corresponding type exports (AreaChartOptions, PieSlice, PieChartOptions,
and GaugeOptions). Remove all conflict markers including the <<<<<<< HEAD,
=======, and >>>>>>> upstream/main lines.
In `@packages/core/src/index.ts`:
- Around line 97-115: Remove the unresolved git conflict markers (<<<<<<< HEAD,
=======, >>>>>>> upstream/main) from the export block. The duplicate exports for
writeClipboard, readClipboard, clipboard, debounce, DebounceOptions, and the
Session module (appearing in both lines 97-100 and 101-104) should be
consolidated into a single export statement that includes stripAnsiControl from
the second occurrence. Keep all other unique exports including throttle,
ThrottleOptions, CommandHistory, CommandHistoryOptions, errors.js exports, and
a11y/index.js exports in their intended order.
In `@packages/core/src/terminal/Screen.ts`:
- Around line 446-494: The Screen.ts file has unresolved merge conflict markers
(<<<<<<< HEAD, =======, >>>>>>> upstream/main) embedded within the exportANSI()
and exportSVG() method definitions, making the file syntactically invalid.
Resolve this conflict by accepting the upstream/main version which has the
correct 4-space indentation for the class method bodies, then remove all
conflict markers ensuring both exportANSI() and exportSVG() methods in the
Screen class are complete and properly indented with consistent formatting.
In `@packages/ui/src/CommandPalette.ts`:
- Around line 129-151: Remove the merge conflict markers in the grouped Map
initialization and keep the upstream version that limits iteration to the vis
slice (using this._maxVisible) rather than all this._filtered commands.
Additionally, locate any instances where the Unicode character ❯ appears as a
raw string for the row prefix and replace it with the conditional expression
caps.unicode ? '❯ ' : '> ' to provide an ASCII fallback (>) for non-Unicode
terminals. Apply the same conflict resolution and Unicode fallback fix to the
second conflict section at lines 185-202 to ensure consistent handling
throughout CommandPalette.
In `@packages/widgets/src/data/Table.ts`:
- Around line 139-171: Resolve the merge conflict markers in the handleKey
method by keeping the upstream/main version that uses handleKey(event: KeyEvent)
parameter, which is the correct widget integration contract. Additionally, add a
guard condition in the down-key path to prevent selectedRow from becoming -1
when the table is empty; check if this._rows.length is greater than zero before
allowing the selectedRow to be updated on the down key press to ensure empty
tables cannot publish an invalid selectedRow state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f5a89bd7-bec2-412f-871b-1f45727610b7
📒 Files selected for processing (12)
packages/charts/package.jsonpackages/charts/src/index.tspackages/core/src/index.tspackages/core/src/session/Session.tspackages/core/src/terminal/Screen.tspackages/store/src/index.tspackages/ui/src/CommandPalette.tspackages/ui/src/index.tspackages/widgets/src/data/Table.tspackages/widgets/src/display/Markdown.tspackages/widgets/src/index.tspackages/widgets/src/input/DatePicker.ts
| export class DatePicker { | ||
| private currentDate: Date; | ||
| private selectedDate: Date | null; | ||
| private format: string; | ||
| private onSelect?: (date: Date) => void; | ||
|
|
||
| constructor(options: DatePickerOptions = {}) { | ||
| this.currentDate = new Date(); | ||
| this.selectedDate = options.selectedDate ?? null; | ||
| this.format = options.format ?? "DD/MM/YYYY"; | ||
| this.onSelect = options.onSelect; | ||
| } | ||
|
|
||
| getCurrentMonth(): number { | ||
| return this.currentDate.getMonth() + 1; | ||
| } | ||
|
|
||
| getCurrentYear(): number { | ||
| return this.currentDate.getFullYear(); | ||
| } | ||
|
|
||
| nextMonth(): void { | ||
| this.currentDate.setMonth(this.currentDate.getMonth() + 1); | ||
| } | ||
|
|
||
| previousMonth(): void { | ||
| this.currentDate.setMonth(this.currentDate.getMonth() - 1); | ||
| } | ||
|
|
||
| selectDate(date: Date): void { | ||
| this.selectedDate = date; | ||
|
|
||
| if (this.onSelect) { | ||
| this.onSelect(date); | ||
| } | ||
| } | ||
|
|
||
| getSelectedDate(): Date | null { | ||
| return this.selectedDate; | ||
| } | ||
|
|
||
| formatDate(): string { | ||
| if (!this.selectedDate) { | ||
| return ""; | ||
| } | ||
|
|
||
| return this.selectedDate.toLocaleDateString(); | ||
| } | ||
|
|
||
| getCalendarDays(): number[] { | ||
| const year = this.currentDate.getFullYear(); | ||
| const month = this.currentDate.getMonth(); | ||
|
|
||
| return Array.from( | ||
| { length: new Date(year, month + 1, 0).getDate() }, | ||
| (_, index) => index + 1 | ||
| ); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
DatePicker is not implemented as a widget contract, so it won’t integrate with the widgets runtime.
This class is currently a plain data helper. In this package layer, widget files are expected to extend Widget, implement _renderSelf(screen: Screen), and dirty-mark visible state changes. Without that, it cannot participate correctly in rendering/input lifecycle.
Suggested direction
-export class DatePicker {
+export class DatePicker extends Widget {
private currentDate: Date;
private selectedDate: Date | null;
- private format: string;
+ private format: string;
private onSelect?: (date: Date) => void;
- constructor(options: DatePickerOptions = {}) {
- this.currentDate = new Date();
+ constructor(label?: string, style = {}, opts: DatePickerOptions = {}) {
+ super(label, style, opts);
+ this.currentDate = new Date();
- this.selectedDate = options.selectedDate ?? null;
- this.format = options.format ?? "DD/MM/YYYY";
- this.onSelect = options.onSelect;
+ this.selectedDate = opts.selectedDate ?? null;
+ this.format = opts.format ?? "DD/MM/YYYY";
+ this.onSelect = opts.onSelect;
}
nextMonth(): void {
this.currentDate.setMonth(this.currentDate.getMonth() + 1);
+ this.markDirty();
}
previousMonth(): void {
this.currentDate.setMonth(this.currentDate.getMonth() - 1);
+ this.markDirty();
}
selectDate(date: Date): void {
this.selectedDate = date;
+ this.markDirty();
if (this.onSelect) this.onSelect(date);
}
+ protected _renderSelf(screen: Screen): void {
+ const { x, y, width, height } = this._getContentRect();
+ if (width <= 0 || height <= 0) return;
+ // draw calendar cells...
+ }
}As per coding guidelines, widget files under packages/widgets/src/+(display|data|input)/*.ts must extend Widget, implement _renderSelf, and call this.markDirty() for visible state mutations.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/widgets/src/input/DatePicker.ts` around lines 7 - 65, The DatePicker
class must extend the Widget base class to integrate with the widgets runtime.
Change the class declaration to inherit from Widget, then implement the required
_renderSelf method that accepts a Screen parameter to define how the DatePicker
renders. Additionally, any method that modifies visible state such as nextMonth,
previousMonth, and selectDate should call this.markDirty() after updating the
state to notify the widget runtime that re-rendering is needed.
Source: Coding guidelines
| constructor(options: DatePickerOptions = {}) { | ||
| this.currentDate = new Date(); | ||
| this.selectedDate = options.selectedDate ?? null; | ||
| this.format = options.format ?? "DD/MM/YYYY"; |
There was a problem hiding this comment.
Initial calendar month does not follow selectedDate.
When an initial selectedDate is provided, currentDate still starts at “today,” so the first rendered month/year can mismatch the selected value.
Small constructor adjustment
constructor(options: DatePickerOptions = {}) {
- this.currentDate = new Date();
+ this.currentDate = options.selectedDate ? new Date(options.selectedDate) : new Date();
this.selectedDate = options.selectedDate ?? null;As per coding guidelines, widget behavior should be coherent across state and rendering flows in changed code paths.
📝 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.
| constructor(options: DatePickerOptions = {}) { | |
| this.currentDate = new Date(); | |
| this.selectedDate = options.selectedDate ?? null; | |
| this.format = options.format ?? "DD/MM/YYYY"; | |
| constructor(options: DatePickerOptions = {}) { | |
| this.currentDate = options.selectedDate ? new Date(options.selectedDate) : new Date(); | |
| this.selectedDate = options.selectedDate ?? null; | |
| this.format = options.format ?? "DD/MM/YYYY"; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/widgets/src/input/DatePicker.ts` around lines 13 - 16, The
DatePicker constructor always initializes currentDate to today's date using new
Date(), regardless of whether a selectedDate option is provided. When a
selectedDate is provided in the options parameter, set currentDate to the
selectedDate value instead of always defaulting to today's date. This ensures
the calendar displays the correct month/year that corresponds to the initial
selection. Modify the assignment of this.currentDate in the constructor to use
options.selectedDate if it exists, otherwise fall back to new Date().
Source: Coding guidelines
| this.format = options.format ?? "DD/MM/YYYY"; | ||
| this.onSelect = options.onSelect; |
There was a problem hiding this comment.
format is exposed but never applied, so public API behavior is incorrect.
format is stored but formatDate() always uses toLocaleDateString(). This makes the format option ineffective.
Minimal fix
formatDate(): string {
if (!this.selectedDate) {
return "";
}
- return this.selectedDate.toLocaleDateString();
+ switch (this.format) {
+ case "YYYY-MM-DD":
+ return this.selectedDate.toISOString().slice(0, 10);
+ case "DD/MM/YYYY":
+ return this.selectedDate.toLocaleDateString("en-GB");
+ case "MM/DD/YYYY":
+ return this.selectedDate.toLocaleDateString("en-US");
+ default:
+ return this.selectedDate.toLocaleDateString();
+ }
}As per coding guidelines, changed segments must satisfy declared behavior cleanly; exposing unused configuration in a public API violates that expectation.
Also applies to: 48-54
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/widgets/src/input/DatePicker.ts` around lines 16 - 17, The format
property is being stored in the DatePicker constructor but is not actually used
by the formatDate() method, which always uses toLocaleDateString() instead.
Update the formatDate() method in the DatePicker class to respect and apply the
this.format value when formatting dates, so the format option provided through
options becomes effective. The method should use the stored format string to
format dates appropriately rather than ignoring it.
Source: Coding guidelines
|
@Karanjot786 lables are missing |
Karanjot786
added
gssoc:approved
2 participants
Summary
Closes #1594
Summary by CodeRabbit
New Features
Changes