Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughAdds a Date Picker system: new DatePicker component (single and range), Calendar and Popover UI primitives, date-presets utilities, documentation and live examples, package dependency additions, public exports, example components, and a small FontAwesome Script loader. ChangesDate Picker Feature
FontAwesome Script Loader
🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Contributor
🍈 Lychee Link Check Report3664 links: ✅ All links are working!Full Statistics Table
|
carlagn
force-pushed
the
feat/DR-7293-date-picker
branch
from
adfcf60 to
c5bf3b1
Compare
carlagn
force-pushed
the
feat/DR-7293-date-picker
branch
from
c5bf3b1 to
dda84bc
Compare
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
Add calendar Add popover Add presets
Update date picker
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
packages/eclipse/src/components/date-picker.tsx (1)
187-201: 💤 Low valueConsider using preset label as the key instead of index.
While the presets array is likely stable, using the
labelas a key is more semantically correct and resilient to potential reordering:♻️ Suggested change
- {presets.map((preset, index) => ( + {presets.map((preset) => ( <Button - key={index} + key={preset.label} variant="default-weaker"🤖 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/eclipse/src/components/date-picker.tsx` around lines 187 - 201, In the presets.map in date-picker.tsx, replace the unstable key={index} with a stable identifier such as key={preset.label} to avoid reordering bugs; update the map callback that renders <Button> (the block that calls onDateRangeChange and setOpen) to use preset.label as the key and, if labels might be non-unique or missing, fall back to a deterministic alternative (e.g., `${preset.label ?? ''}-${index}`) to guarantee uniqueness.apps/eclipse/src/components/date-picker-examples.tsx (1)
98-103: 💤 Low valueConsider using the design system Button component for consistency.
The submit button uses raw HTML with inline Tailwind classes. For documentation examples, using the
Buttoncomponent from the design system would better demonstrate the intended usage patterns.♻️ Suggested change
+import { + DatePickerSingle, + DatePickerRange, + createDateRangePresets, + Button, +} from "@prisma/eclipse"; // ... in the form: - <button - type="submit" - className="px-4 py-2 bg-background-ppg text-foreground-ppg rounded-md hover:bg-background-ppg/90" - > - Submit - </button> + <Button type="submit" variant="primary"> + Submit + </Button>🤖 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 `@apps/eclipse/src/components/date-picker-examples.tsx` around lines 98 - 103, Replace the raw HTML <button> used in date-picker examples with the design system Button component: import Button where needed and render <Button type="submit">Submit</Button> (or use the Button props that map to the current Tailwind styling, e.g., variant/size/intent if your design system exposes them) so the example uses the shared component API and visual styles; preserve the type="submit" and any accessibility attributes (aria-*) when converting and remove the inline Tailwind className.
🤖 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 `@apps/eclipse/content/design-system/components/datepicker.mdx`:
- Around line 228-229: The disabled predicates in the DatePicker examples use
direct timestamp comparisons (disabled={(date) => date < new Date()}) so "today"
can be treated as past; update each predicate (the disabled prop instances in
the examples) to normalize both operands to start-of-day (midnight) before
comparing—for example compute startOfDay(date) and startOfDay(new Date()) (or
call a small helper like startOfDay) and then compare those values; apply this
change to all three occurrences of the disabled prop in this file so the
comparison uses date-only values.
- Around line 186-205: The example's customPresets uses startOfWeek but doesn't
import it; update the snippet to import startOfWeek from date-fns so the example
compiles. Specifically, add an import for startOfWeek alongside any existing
DatePicker/React imports referenced in the snippet so startOfWeek(new Date()) in
customPresets (used by DatePickerRange) resolves correctly.
- Around line 544-549: The example's MyForm component calls
handleSubmit(onSubmit) but never defines onSubmit, causing a runtime
ReferenceError; fix by adding a submission handler named onSubmit in the MyForm
scope (e.g., a function that accepts form data and processes it) or by passing
an inline handler to handleSubmit (e.g., handleSubmit(data => { ... })); update
the snippet around the useForm/Controller block so MyForm declares onSubmit
before it is referenced.
In `@packages/eclipse/src/components/date-picker.tsx`:
- Around line 68-75: The JSDoc for the dateFormat prop is inconsistent with the
implemented default; update the JSDoc so the `@default` for single mode matches
the actual default used in the code (change `@default` "PPP" to `@default` "P"),
keep the range-mode `@default` "LLL dd, y" and existing examples as appropriate,
and ensure the JSDoc references the same symbol name dateFormat used in the
component so documentation and implementation align.
- Around line 126-136: The Calendar component uses the removed react-day-picker
v9 prop initialFocus; replace initialFocus with autoFocus in both Calendar
instances (the one inside PopoverContent and the other occurrence around lines
~204-211) so the Calendar props include autoFocus instead of initialFocus;
update the Calendar JSX in the date-picker component accordingly (look for the
Calendar elements with props mode="single", selected={date}, onSelect and
disabled={disabled} to locate each instance).
In `@packages/eclipse/src/components/fontawesome-script.tsx`:
- Around line 28-35: The FontAwesomeScript component currently hard-codes the
kit URL; change FontAwesomeScript to accept a kitUrl prop (e.g., function
FontAwesomeScript({ kitUrl }: { kitUrl?: string })) and use that for the Script
src, with a safe fallback to an environment variable like
process.env.NEXT_PUBLIC_FONT_AWESOME_KIT; also validate/panic with a clear error
or skip rendering if neither prop nor env var is provided so consumers must
supply their own kit URL. Ensure you only change the FontAwesomeScript
export/usage (update any internal calls to pass the prop) and keep
crossOrigin/async/strategy unchanged.
In `@packages/eclipse/src/lib/date-presets.ts`:
- Around line 22-49: The preset date calculations in date-presets.ts that
compute from via new Date(today.getTime() - N * 24 * 60 * 60 * 1000) are
vulnerable to DST errors; replace those millisecond arithmetic usages for the
"Last 7 days", "Last 14 days", "Last 30 days", and "Last 90 days" presets with
date-fns helpers (e.g. import and use subDays and optionally startOfDay) to
compute the from/to ranges (use startOfDay(today) for consistent boundaries and
subDays(startOfDay(today), N-1) or subDays(today, N-1) as appropriate) so DST
transitions are handled correctly.
---
Nitpick comments:
In `@apps/eclipse/src/components/date-picker-examples.tsx`:
- Around line 98-103: Replace the raw HTML <button> used in date-picker examples
with the design system Button component: import Button where needed and render
<Button type="submit">Submit</Button> (or use the Button props that map to the
current Tailwind styling, e.g., variant/size/intent if your design system
exposes them) so the example uses the shared component API and visual styles;
preserve the type="submit" and any accessibility attributes (aria-*) when
converting and remove the inline Tailwind className.
In `@packages/eclipse/src/components/date-picker.tsx`:
- Around line 187-201: In the presets.map in date-picker.tsx, replace the
unstable key={index} with a stable identifier such as key={preset.label} to
avoid reordering bugs; update the map callback that renders <Button> (the block
that calls onDateRangeChange and setOpen) to use preset.label as the key and, if
labels might be non-unique or missing, fall back to a deterministic alternative
(e.g., `${preset.label ?? ''}-${index}`) to guarantee uniqueness.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d10cc91b-8105-4278-b05d-4e68e282518d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
apps/eclipse/content/design-system/components/datepicker.mdxapps/eclipse/src/components/date-picker-examples.tsxpackages/eclipse/package.jsonpackages/eclipse/src/components/date-picker.tsxpackages/eclipse/src/components/fontawesome-script.tsxpackages/eclipse/src/components/index.tspackages/eclipse/src/components/ui/calendar.tsxpackages/eclipse/src/components/ui/popover.tsxpackages/eclipse/src/index.tspackages/eclipse/src/lib/date-presets.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@apps/eclipse/content/design-system/components/datepicker.mdx`:
- Around line 193-195: The "Yesterday" example uses Date.now() minus
24*60*60*1000 which fails across DST; change the example so the from and to
values are computed by creating a Date for midnight today and then subtracting
one calendar day via setDate(getDate() - 1) (so both `from` and `to` are that
adjusted Date), ensuring calendar-day arithmetic rather than fixed millisecond
offsets.
In `@packages/eclipse/src/lib/date-presets.ts`:
- Around line 85-90: The getLastNDays(DateRange) helper should validate its days
input: ensure the days parameter is a positive integer (>0) before computing the
range. In the getLastNDays function, check Number.isInteger(days) and days > 0
and if the check fails throw a clear RangeError (or TypeError) with a
descriptive message, so callers cannot get an invalid range; then proceed to use
startOfDay/new Date and subDays as before.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aed2f433-3ec8-47fc-a728-cd72ff3deec5
📒 Files selected for processing (2)
apps/eclipse/content/design-system/components/datepicker.mdxpackages/eclipse/src/lib/date-presets.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/eclipse/src/components/date-picker.tsx`:
- Around line 100-112: The visual "disabled" state (disabledBtn) is only applied
as styling so the DatePicker triggers remain interactive; update the trigger
Button elements (the single-date trigger and the range triggers identified by
the Button components inside date-picker.tsx) to actually disable interaction
when disabledBtn is true by adding the disabled prop (disabled={disabledBtn})
and also include aria-disabled={disabledBtn} for accessibility; keep the
existing className conditional logic (isErrored, className) but ensure the
Button elements reference disabledBtn to prevent opening/selection when
disabled.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ff8460fa-84cb-4886-80b3-64586d33c731
📒 Files selected for processing (1)
packages/eclipse/src/components/date-picker.tsx
Comment on lines
+100
to
+112
| <Button | ||
| variant="default" | ||
| size="lg" | ||
| className={cn( | ||
| "w-full p-1.5 text-left font-normal bg-background-default border-stroke-neutral font-mono text-foreground-neutral", | ||
| !date && "text-foreground-neutral-weak", | ||
| isErrored && "border-stroke-error text-foreground-error", | ||
| disabledBtn && | ||
| "cursor-not-allowed text-foreground-neutral-weaker bg-background-neutral-weak", | ||
| className, | ||
| )} | ||
| type="button" | ||
| > |
Contributor
There was a problem hiding this comment.
disabledBtn is styling-only; the picker is still interactive.
On Line 107 and Line 154, the component looks disabled but neither trigger Button is actually disabled, so users can still open/select dates.
Suggested fix (apply to both single and range trigger buttons)
- <Popover open={open} onOpenChange={setOpen}>
+ <Popover
+ open={open}
+ onOpenChange={(nextOpen) => {
+ if (!disabledBtn) setOpen(nextOpen);
+ }}
+ >
...
- <Button
+ <Button
+ disabled={disabledBtn}
variant="default"
size="lg"Also applies to: 147-159
🤖 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/eclipse/src/components/date-picker.tsx` around lines 100 - 112, The
visual "disabled" state (disabledBtn) is only applied as styling so the
DatePicker triggers remain interactive; update the trigger Button elements (the
single-date trigger and the range triggers identified by the Button components
inside date-picker.tsx) to actually disable interaction when disabledBtn is true
by adding the disabled prop (disabled={disabledBtn}) and also include
aria-disabled={disabledBtn} for accessibility; keep the existing className
conditional logic (isErrored, className) but ensure the Button elements
reference disabledBtn to prevent opening/selection when disabled.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add calendar
Add popover
Add presets
Summary by CodeRabbit
New Features
Documentation
Testing
Testing Instructions — DatePicker Component
Single Date Picker
Date Range Picker
01/04/2025 - 30/04/2025)Date Range with Presets
Error State
isErroredis trueDisabled Button State
disabledBtnis truenot-allowedon hoverGeneral / Visual
Escape