Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(DatePicker): lib update and related changes #1455

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

marcinsawicki
Copy link
Contributor

@marcinsawicki marcinsawicki commented Dec 17, 2024

Resolves: #{issue-number}

Description

react-day-picker lib update, component adjustment to new interface, and UI update.

Storybook

https://feature-day-picker-lib-update--613a8e945a5665003a05113b.chromatic.com

Checklist

Obligatory:

  • Self review (use this as your final check for proposed changes before requesting the review)
  • Add correct label
  • Assign pull request with the correct issue

Summary by CodeRabbit

  • New Features

    • Updated the react-day-picker dependency to a newer version.
    • Introduced a new DatePickerCustomNavigation component for enhanced navigation.
    • Added new stories for the DatePicker component, including WithCustomCurrentDate and WithDatesBetweenTwoMonths.
  • Bug Fixes

    • Adjusted prop names in tests for consistency with updated component API.
  • Style

    • Updated styles for the DatePicker component to improve layout and user interaction.
    • Introduced new styles for the DatePickerCustomNavigation component.
  • Refactor

    • Streamlined the DatePicker component's implementation and internal logic.
    • Simplified type definitions for the DatePicker component.
  • Chores

    • Removed unused components and helper functions to clean up the codebase.

@marcinsawicki marcinsawicki added feature New feature or request dependencies Pull requests that update a dependency file labels Dec 17, 2024
@marcinsawicki marcinsawicki added this to the Cycle #10 milestone Dec 17, 2024
@marcinsawicki marcinsawicki self-assigned this Dec 17, 2024
Copy link
Contributor

coderabbitai bot commented Dec 17, 2024

Walkthrough

The pull request introduces an upgrade to the DatePicker component in the React Components library, primarily migrating from react-day-picker version 7 to version 9. This involves extensive changes to the component's implementation, including updates to styling, type definitions, stories, and internal logic. The modifications enhance the DatePicker's functionality, flexibility, and customization capabilities.

Changes

File Change Summary
packages/react-components/package.json Updated react-day-picker dependency from ^7.4.10 to ^9.4.2
packages/react-components/src/components/DatePicker/DatePicker.mdx Updated story references from DatePickerStories.DatePicker to DatePickerStories.Default
packages/react-components/src/components/DatePicker/DatePicker.module.scss Significant styling overhaul with flexbox adjustments and enhanced interaction states
packages/react-components/src/components/DatePicker/DatePicker.spec.tsx Updated prop name from firstDayOfWeek to weekStartsOn in test cases
packages/react-components/src/components/DatePicker/DatePicker.stories.tsx Restructured stories with new export methods and simplified definitions
packages/react-components/src/components/DatePicker/DatePicker.tsx Refactored component implementation using new react-day-picker API
packages/react-components/src/components/DatePicker/helpers.ts Removed getRangeDatePickerModifiers and getDatePickerClassNames functions
packages/react-components/src/components/DatePicker/types.ts Updated type definitions to align with new react-day-picker version
packages/react-components/src/components/DatePicker/RangeDatePicker.tsx Modified date selection logic and API structure for range selection
packages/react-components/src/components/DatePicker/components/DatePickerCustomNavigation.module.scss Introduced styles for DatePickerCustomNavigation component
packages/react-components/src/components/DatePicker/components/DatePickerCustomNavigation.tsx Added DatePickerCustomNavigation component for custom navigation
packages/react-components/src/components/DatePicker/DatePickerNavbar.tsx Removed DatePickerNavbar component
packages/react-components/src/components/DatePicker/RangeDatePicker.spec.tsx Added utility function formattedDate for date formatting in tests

Suggested labels

bug

Suggested reviewers

  • vladko-uxds
  • ashbork

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (11)
packages/react-components/src/components/DatePicker/DatePicker.stories.tsx (1)

12-25: Consider adding stories for error states and edge cases

The current stories cover basic usage but miss important scenarios like:

  • Invalid date ranges
  • Disabled dates
  • Loading states

Would you like me to provide example implementations for these scenarios?

packages/react-components/src/components/DatePicker/helpers.ts (5)

1-7: Consider grouping imports by source

Group external and internal imports for better organization.

-import { isAfter, isSameDay, isSameMonth, subMonths } from 'date-fns';
-
-import {
-  IRangeDatePickerProps,
-  IRangeDatePickerState,
-  IRangeDatePickerOption,
-} from './types';
+// External imports
+import { isAfter, isSameDay, isSameMonth, subMonths } from 'date-fns';
+
+// Internal imports
+import {
+  IRangeDatePickerProps,
+  IRangeDatePickerState,
+  IRangeDatePickerOption,
+} from './types';

Line range hint 9-22: Simplify redundant if statement

The function can be more concise by combining conditions.

 export const isDateWithinRange = (
   date: Date,
   range: { from?: Date; to?: Date }
 ): boolean => {
   const { from, to } = range;
-  if (to && !isSameDay(date, to) && isAfter(date, to)) {
-    return false;
-  }
-  // noinspection RedundantIfStatementJS
-  if (from && !isSameDay(date, from) && !isAfter(date, from)) {
-    return false;
-  }
-
-  return true;
+  return !(
+    (to && !isSameDay(date, to) && isAfter(date, to)) ||
+    (from && !isSameDay(date, from) && !isAfter(date, from))
+  );
 };

Line range hint 24-40: Improve variable naming for clarity

The variable names could be more descriptive to better convey their purpose.

 export const calculateDatePickerMonth = (
   initialFromDate?: Date,
   initialToDate?: Date,
   toMonth?: Date
 ): Date => {
   if (!initialToDate) {
     return subMonths(toMonth || new Date(), 1);
   }
 
-  const forcePreviousMonth =
+  const shouldShowPreviousMonth =
     initialFromDate && !isSameMonth(initialFromDate, initialToDate);
 
-  if (forcePreviousMonth || (toMonth && isSameMonth(initialToDate, toMonth))) {
+  if (shouldShowPreviousMonth || (toMonth && isSameMonth(initialToDate, toMonth))) {
     return subMonths(initialToDate, 1);
   }
 
   return initialToDate;
 };

Line range hint 42-50: Simplify undefined return

The void 0 pattern is unnecessary here.

 export const getSelectedOption = (
   itemId: string | null,
   options: IRangeDatePickerOption[]
 ): IRangeDatePickerOption | undefined => {
-  const selectedOption = options.find((item) => {
-    return item.id === itemId;
-  });
-
-  return selectedOption ? selectedOption : void 0;
+  return options.find(item => item.id === itemId);
 };

Line range hint 58-93: Enhance type safety

Consider using more specific types and adding null checks.

 export const getInitialStateFromProps = (
   props: IRangeDatePickerProps
 ): Partial<IRangeDatePickerState> => {
   const state: Partial<IRangeDatePickerState> = {};
 
-  if (!props.initialSelectedItemKey) {
+  if (props.initialSelectedItemKey == null) {
     return state;
   }
 
   const selectedOption = getSelectedOption(
     props.initialSelectedItemKey,
     props.options
   );
 
-  if (!selectedOption) {
+  if (selectedOption == null) {
     return {};
   }
 
   state.selectedItem = props.initialSelectedItemKey;
 
-  if (!selectedOption.isManual) {
+  if (selectedOption.isManual !== true) {
     return state;
   }
 
-  if (props.initialFromDate) {
+  if (props.initialFromDate != null) {
     state.from = props.initialFromDate;
   }
-  if (props.initialToDate) {
+  if (props.initialToDate != null) {
     state.to = props.initialToDate;
     state.temporaryTo = props.initialToDate;
   }
 
   return state;
 };
packages/react-components/src/components/DatePicker/DatePicker.module.scss (1)

21-21: Specify Transition Properties Instead of 'all'

Using transition: all may lead to unintended transitions. It's better to specify only the needed properties.

Apply this diff to specify the transitioned property:

-        transition: all var(--transition-duration-fast-2) ease-in-out;
+        transition: background-color var(--transition-duration-fast-2) ease-in-out;
packages/react-components/src/components/DatePicker/DatePicker.tsx (1)

18-26: Consolidate Props for Clarity

Consider merging fromMonth with startMonth and toMonth with endMonth to reduce redundancy.

packages/react-components/src/components/DatePicker/components/DatePickerCustomNavigation.tsx (2)

32-42: Use date-fns consistently for date manipulation

Replace manual date manipulation with date-fns functions for consistency.

-    const newMonth = new Date(currentMonth);
-    newMonth.setMonth(currentMonth.getMonth() - 1);
+    const newMonth = subMonths(currentMonth, 1);

73-75: Add comment explaining the special case

The adjustment for two-month display needs explanation.

+    // When displaying two months, prevent the first month from being the same as the end month
     if (numberOfMonths === 2 && isSameMonth(newMonth, endMonth)) {
       return setMonth(subMonths(newMonth, 1));
     }
packages/react-components/src/components/DatePicker/DatePicker.spec.tsx (1)

Line range hint 60-68: Add more test cases for weekStartsOn

Test should verify other weekday options and edge cases.

it('should accept custom weekStartsOn value', () => {
  const { getAllByRole } = renderComponent({ weekStartsOn: 0 });
  const weekdays = getAllByRole('columnheader');
  const firstWeekday = within(weekdays[0]).getByTitle('Sunday');
  expect(firstWeekday).toBeDefined();
});
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8736e71 and 2ebf49f.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (12)
  • packages/react-components/package.json (1 hunks)
  • packages/react-components/src/components/DatePicker/DatePicker.mdx (2 hunks)
  • packages/react-components/src/components/DatePicker/DatePicker.module.scss (1 hunks)
  • packages/react-components/src/components/DatePicker/DatePicker.spec.tsx (1 hunks)
  • packages/react-components/src/components/DatePicker/DatePicker.stories.tsx (1 hunks)
  • packages/react-components/src/components/DatePicker/DatePicker.tsx (2 hunks)
  • packages/react-components/src/components/DatePicker/DatePickerNavbar.tsx (0 hunks)
  • packages/react-components/src/components/DatePicker/RangeDatePicker.tsx (3 hunks)
  • packages/react-components/src/components/DatePicker/components/DatePickerCustomNavigation.module.scss (1 hunks)
  • packages/react-components/src/components/DatePicker/components/DatePickerCustomNavigation.tsx (1 hunks)
  • packages/react-components/src/components/DatePicker/helpers.ts (1 hunks)
  • packages/react-components/src/components/DatePicker/types.ts (2 hunks)
💤 Files with no reviewable changes (1)
  • packages/react-components/src/components/DatePicker/DatePickerNavbar.tsx
✅ Files skipped from review due to trivial changes (1)
  • packages/react-components/src/components/DatePicker/components/DatePickerCustomNavigation.module.scss
🧰 Additional context used
🪛 eslint
packages/react-components/src/components/DatePicker/DatePicker.tsx

[error] 12-12: Unexpected use of file extension "css" for "react-day-picker/style.css"

(import/extensions)

🔇 Additional comments (13)
packages/react-components/src/components/DatePicker/DatePicker.stories.tsx (1)

3-10: LGTM! Clean and well-structured configuration

The story configuration follows Storybook best practices with proper typing.

packages/react-components/src/components/DatePicker/helpers.ts (1)

Line range hint 52-56: LGTM!

The function is concise and its logic is clear.

packages/react-components/src/components/DatePicker/DatePicker.module.scss (3)

5-11: Layout Enhancements Look Good

The updates to the &__wrapper class improve the layout and alignment.


32-49: Accessible Button Styling Approved

Introducing &__day-button enhances focus states and accessibility.


83-90: Efficient Use of Pseudo-Elements

Great use of &::after for range styling in &__range-middle.

packages/react-components/src/components/DatePicker/DatePicker.tsx (1)

80-87: ⚠️ Potential issue

Pass Props to Custom Navigation

The Nav component should accept props to ensure proper functionality.

Update the Nav definition:

-          Nav: () => (
+          Nav: (props) => (
               <DatePickerCustomNavigation
-                currentMonth={currentMonth}
-                setMonth={handleMonthChange}
-                startMonth={startMonth || fromMonth}
-                endMonth={endMonth || toMonth}
+                {...props}
               />
             ),

Likely invalid or redundant comment.

packages/react-components/src/components/DatePicker/DatePicker.mdx (2)

11-11: Story Reference Updated Correctly

The change to DatePickerStories.Default is appropriate.


25-25: ArgTypes Reference Updated

Updating to DatePickerStories.Default ensures consistency.

packages/react-components/src/components/DatePicker/types.ts (2)

5-5: Simplified Props Type

Extending DayPickerProps directly cleans up IDatePickerProps.


7-11: Custom Navigation Props Defined Well

IDatePickerCustomNavigationProps accurately captures necessary props.

packages/react-components/package.json (1)

85-85: Verify breaking changes from react-day-picker v7 to v9

Major version upgrade requires careful review of breaking changes.

packages/react-components/src/components/DatePicker/RangeDatePicker.tsx (2)

3-3: Verify date-fns import usage

Let's ensure all imported date-fns functions are necessary and not duplicated elsewhere.

✅ Verification successful

All date-fns imports are actively used

Each imported function from date-fns is used in the component:

  • differenceInCalendarDays: Used for date range validation
  • isSameDay: Used for day comparison
  • isAfter: Used for range boundary check
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usage of date-fns functions
rg --type typescript "isAfter|isSameDay|differenceInCalendarDays" packages/react-components/src/components/DatePicker/

Length of output: 155


Script:

#!/bin/bash
# Search for usage of date-fns functions without type filter
rg "isAfter|isSameDay|differenceInCalendarDays" packages/react-components/src/components/DatePicker/RangeDatePicker.tsx

Length of output: 327


Line range hint 208-231: API structure updated for react-day-picker v9

The changes align with v9's API requirements:

  • Object-based selection format
  • Explicit range mode
  • Month navigation controls
✅ Verification successful

API changes for react-day-picker v9 are properly implemented

The changes are consistent across the codebase:

  • Types are properly aligned with v9's DayPickerProps
  • No legacy v7 API patterns found
  • Stories and implementation use the new API format
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining v7 API patterns that need updating
ast-grep --pattern 'selected = { from: $_, to: $_ }' packages/react-components/src/components/DatePicker/
rg --type typescript "range: true|initialMonth:|toMonth:" packages/react-components/src/components/DatePicker/

Length of output: 255


Script:

#!/bin/bash
# Search for any remaining v7 API patterns that need updating
rg -t ts "range: true|initialMonth:|toMonth:" packages/react-components/src/components/DatePicker/
rg -t tsx "range: true|initialMonth:|toMonth:" packages/react-components/src/components/DatePicker/

# Search for other DatePicker components that might need updates
ast-grep --pattern 'DayPicker {
  $$$
}'

# Look for any other range-related props
rg -t tsx "mode.*range|selected.*from.*to" packages/react-components/src/components/DatePicker/

Length of output: 494


Script:

#!/bin/bash
# Search for files first
fd -e tsx -e ts . packages/react-components/src/components/DatePicker/ -x echo {}

# Then use these file paths to search for patterns
find packages/react-components/src/components/DatePicker/ -name "*.tsx" -o -name "*.ts" -exec rg -l "DayPicker|mode.*range|selected.*from.*to|range: true|initialMonth:|toMonth:" {} \;

# Get content of stories file that had a match
cat packages/react-components/src/components/DatePicker/RangeDatePicker.stories.tsx

Length of output: 3612


Script:

#!/bin/bash
# Check types.ts for any v7 API definitions
cat packages/react-components/src/components/DatePicker/types.ts

# Look for any other DayPicker usage
rg -g '*.{ts,tsx}' "DayPicker" packages/react-components/src/components/DatePicker/

Length of output: 3039

Comment on lines 14 to 22
export const WithCustomCurrentDate = () => (
<DatePicker today={new Date('01/20/2024')} />
);
DatePickerWithDatesBetweenTwoMonths.args = {
fromMonth: new Date('06/20/2021'),
toMonth: new Date('09/20/2021'),
month: new Date('09/20/2021'),
};

export const DatePickerWithCustomFirstDayOfWeek: StoryFn = StoryTemplate.bind(
{}
export const WithDatesBetweenTwoMonths = () => (
<DatePicker
startMonth={new Date('11/20/2023')}
endMonth={new Date('03/20/2024')}
/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider using relative dates instead of hardcoded ones

The hardcoded dates will become outdated. Consider using dynamic dates relative to the current date.

export const WithCustomCurrentDate = () => (
-  <DatePicker today={new Date('01/20/2024')} />
+  <DatePicker today={new Date(Date.now() + 86400000)} /> {/* tomorrow */}
);

export const WithDatesBetweenTwoMonths = () => {
+  const today = new Date();
+  const startMonth = new Date(today.getFullYear(), today.getMonth() - 1, 1);
+  const endMonth = new Date(today.getFullYear(), today.getMonth() + 2, 1);
+  return (
    <DatePicker
-    startMonth={new Date('11/20/2023')}
-    endMonth={new Date('03/20/2024')}
+    startMonth={startMonth}
+    endMonth={endMonth}
    />
+  );
};
📝 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.

Suggested change
export const WithCustomCurrentDate = () => (
<DatePicker today={new Date('01/20/2024')} />
);
DatePickerWithDatesBetweenTwoMonths.args = {
fromMonth: new Date('06/20/2021'),
toMonth: new Date('09/20/2021'),
month: new Date('09/20/2021'),
};
export const DatePickerWithCustomFirstDayOfWeek: StoryFn = StoryTemplate.bind(
{}
export const WithDatesBetweenTwoMonths = () => (
<DatePicker
startMonth={new Date('11/20/2023')}
endMonth={new Date('03/20/2024')}
/>
export const WithCustomCurrentDate = () => (
<DatePicker today={new Date(Date.now() + 86400000)} /> {/* tomorrow */}
);
export const WithDatesBetweenTwoMonths = () => {
const today = new Date();
const startMonth = new Date(today.getFullYear(), today.getMonth() - 1, 1);
const endMonth = new Date(today.getFullYear(), today.getMonth() + 2, 1);
return (
<DatePicker
startMonth={startMonth}
endMonth={endMonth}
/>
);
};

Comment on lines +88 to +89
disabled={!shouldDisablePreviousButton}
/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix disabled prop values

The disabled props are using negated values which is confusing.

-          disabled={!shouldDisablePreviousButton}
+          disabled={shouldDisablePreviousButton}

Also applies to: 95-96, 104-105, 111-112

Comment on lines +27 to +30
const shouldDisablePreviousButton =
!startMonth || !isSameMonth(currentMonth, startMonth);
const shouldDisableNextButton =
!endMonth || !isSameMonth(currentMonth, endMonth);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix incorrect button disable logic

The disable conditions are inverted. shouldDisableButton should be true when the month IS same as boundary month.

-  const shouldDisablePreviousButton =
-    !startMonth || !isSameMonth(currentMonth, startMonth);
-  const shouldDisableNextButton =
-    !endMonth || !isSameMonth(currentMonth, endMonth);
+  const shouldDisablePreviousButton =
+    startMonth && isSameMonth(currentMonth, startMonth);
+  const shouldDisableNextButton =
+    endMonth && isSameMonth(currentMonth, endMonth);
📝 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.

Suggested change
const shouldDisablePreviousButton =
!startMonth || !isSameMonth(currentMonth, startMonth);
const shouldDisableNextButton =
!endMonth || !isSameMonth(currentMonth, endMonth);
const shouldDisablePreviousButton =
startMonth && isSameMonth(currentMonth, startMonth);
const shouldDisableNextButton =
endMonth && isSameMonth(currentMonth, endMonth);

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
packages/react-components/src/components/DatePicker/DatePicker.tsx (1)

59-73: Consider extracting classNames configuration

The classNames object is quite large. Consider moving it to a separate constant.

+const datePickerClassNames = {
+  weekday: cx(
+    styles[`${baseClass}__wrapper`],
+    styles[`${baseClass}__weekday`]
+  ),
+  // ... rest of the classNames
+};

 return (
   <DayPicker
-    classNames={{...}}
+    classNames={datePickerClassNames}
packages/react-components/src/components/DatePicker/DatePicker.stories.tsx (1)

84-93: Consider using an enum for disabled date options

Replace the string array with an enum to improve type safety and maintainability.

+enum DisabledRangeOption {
+  None = 'No Disabled Dates',
+  BeforeCurrent = 'Disable Before Current Date',
+  AfterCurrent = 'Disable After Current Date',
+}

-const DISABLED_RANGE_OPTIONS = [
-  'No Disabled Dates',
-  'Disable Before Current Date',
-  'Disable After Current Date',
-];
+const DISABLED_RANGE_OPTIONS = Object.values(DisabledRangeOption);

const getDisabledDates = () => {
  switch (restArgs.disabled as unknown) {
-    case DISABLED_RANGE_OPTIONS[1]:
+    case DisabledRangeOption.BeforeCurrent:
      return { before: new Date() };
-    case DISABLED_RANGE_OPTIONS[2]:
+    case DisabledRangeOption.AfterCurrent:
      return { after: new Date() };
    default:
      return undefined;
  }
};
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2ebf49f and 1abb2fe.

📒 Files selected for processing (6)
  • packages/react-components/src/components/DatePicker/DatePicker.module.scss (1 hunks)
  • packages/react-components/src/components/DatePicker/DatePicker.spec.tsx (3 hunks)
  • packages/react-components/src/components/DatePicker/DatePicker.stories.tsx (2 hunks)
  • packages/react-components/src/components/DatePicker/DatePicker.tsx (2 hunks)
  • packages/react-components/src/components/DatePicker/RangeDatePicker.spec.tsx (3 hunks)
  • packages/react-components/src/components/DatePicker/components/DatePickerCustomNavigation.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/react-components/src/components/DatePicker/DatePicker.spec.tsx
  • packages/react-components/src/components/DatePicker/components/DatePickerCustomNavigation.tsx
  • packages/react-components/src/components/DatePicker/DatePicker.module.scss
🧰 Additional context used
🪛 eslint
packages/react-components/src/components/DatePicker/DatePicker.tsx

[error] 12-12: Unexpected use of file extension "css" for "react-day-picker/style.css"

(import/extensions)

🔇 Additional comments (4)
packages/react-components/src/components/DatePicker/RangeDatePicker.spec.tsx (1)

14-14: LGTM: Good use of date-fns for consistent date formatting

The formattedDate utility improves test readability.

packages/react-components/src/components/DatePicker/DatePicker.tsx (2)

18-27: Props interface aligns with react-day-picker v9

Good job updating the props to match the new library version. The naming is more intuitive (e.g., weekStartsOn).


76-90: Good use of custom components for consistent styling

The custom navigation and caption components maintain UI consistency.

packages/react-components/src/components/DatePicker/DatePicker.stories.tsx (1)

111-114: Use relative dates instead of hardcoded ones

The hardcoded dates will become outdated. Consider using dynamic dates relative to the current date.

export const WithCustomCurrentDate: StoryFn = StoryTemplate.bind({});
WithCustomCurrentDate.args = {
-  today: new Date('01/20/2024'),
+  today: new Date(Date.now() + 86400000), // tomorrow
};

export const WithDatesBetweenTwoMonths: StoryFn = StoryTemplate.bind({});
WithDatesBetweenTwoMonths.args = {
-  startMonth: new Date('11/20/2023'),
-  endMonth: new Date('01/20/2024'),
+  startMonth: new Date(new Date().setMonth(new Date().getMonth() - 1)),
+  endMonth: new Date(new Date().setMonth(new Date().getMonth() + 1)),
};

Also applies to: 116-119

Comment on lines +40 to 45
toMonth: new Date(2022, 1, 1),
onChange,
options,
initialSelectedItemKey: 'custom_date',
children,
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Additional test cases needed for RangeDatePicker

The current test suite only covers the basic custom date selection scenario. Missing test cases for:

  • Invalid date ranges
  • Edge dates (min/max boundaries)
  • Different month selections
  • Pre-selected date ranges
  • Disabled dates
🔗 Analysis chain

Verify test coverage with new props

The test now uses toMonth instead of initialFromDate/initialToDate. Ensure all edge cases are covered.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check test coverage for date range scenarios
rg -A 5 "toMonth|fromMonth|startMonth|endMonth" "**/*.spec.tsx"

Length of output: 119


Script:

#!/bin/bash
# Let's try finding the test files first
fd ".spec.tsx$"

# Then search for date-related props in test files
rg -A 5 "toMonth|fromMonth|startMonth|endMonth" $(fd ".spec.tsx$")

Length of output: 12283


Script:

#!/bin/bash
# Let's check the full content of RangeDatePicker test file to see test coverage
cat packages/react-components/src/components/DatePicker/RangeDatePicker.spec.tsx

Length of output: 1646

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/react-components/src/components/DatePicker/RangeDatePicker.tsx (1)

126-131: Consider adding error handling

The onSelect callback could benefit from try-catch error handling to gracefully handle any callback errors.

 if (from && to && onSelect) {
+  try {
     onSelect({
       from,
       to,
     });
+  } catch (error) {
+    console.error('Error in date selection callback:', error);
+  }
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1abb2fe and ae83adb.

📒 Files selected for processing (2)
  • packages/react-components/src/components/DatePicker/RangeDatePicker.tsx (5 hunks)
  • packages/react-components/src/components/DatePicker/types.ts (3 hunks)
🔇 Additional comments (9)
packages/react-components/src/components/DatePicker/types.ts (5)

3-3: LGTM: Clean import of new types

Direct import of essential types from react-day-picker v9.


84-84: LGTM: Simplified datepicker prop type

Clean simplification using DayPickerProps directly.


5-5: Verify the removal of excluded props

The type now directly extends DayPickerProps without exclusions. Confirm if previously excluded props (todayButton, showWeekNumbers) should still be available.

✅ Verification successful

Direct extension of DayPickerProps is acceptable

No usage of todayButton or showWeekNumbers props was found in the codebase, suggesting these props weren't actively used. The direct extension of DayPickerProps appears to be a safe refactoring choice.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usage of previously excluded props
rg -l 'todayButton|showWeekNumbers' --type ts

Length of output: 47


Script:

#!/bin/bash
# Let's check the git history for these props and their exclusions
git log -S "todayButton|showWeekNumbers" --all -- "packages/react-components/src/components/DatePicker/*"

# Also search for any DayPickerProps type usage
rg "DayPickerProps" --type ts -A 2 -B 2

Length of output: 1084


Script:

#!/bin/bash
# Let's check the previous version of the types file
git log -p packages/react-components/src/components/DatePicker/types.ts

Length of output: 0


95-95: Verify onSelect callback implementation

The new optional onSelect callback provides granular date range selection control. Ensure it's properly handled in the component implementation.

✅ Verification successful

Let me search for more specific information about the DatePicker component implementation.


onSelect callback is properly implemented

The callback is correctly handled in RangeDatePicker.tsx where it's:

  • Destructured from props
  • Called with the expected DateRange object when both from/to dates are selected
  • Used within a proper useCallback hook
  • Integrated in the component's event handling system
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Find the component implementation to verify callback handling
ast-grep --pattern 'onSelect?.($$$)'

Length of output: 139


Script:

#!/bin/bash
# Search for DatePicker component implementation and usage of onSelect
ast-grep --pattern 'const DatePicker = $$$'

# Also search for any direct usage of onSelect in DatePicker files
rg "onSelect" packages/react-components/src/components/DatePicker/ -A 2

Length of output: 1978


7-11: Validate navigation prop changes

The navigation interface has been significantly restructured. The new props align with react-day-picker v9's navigation system, but we should verify all consumers are updated.

packages/react-components/src/components/DatePicker/RangeDatePicker.tsx (4)

123-132: Clean implementation of selection handler

Good use of useCallback and null checks. The handler provides a clean interface for date range selection events.


3-3: Verify date-fns imports usage

Let's ensure all imported date-fns functions are necessary.

✅ Verification successful

All imported date-fns functions are actively used in the codebase

  • isAfter and isSameDay are used in both helpers.ts and RangeDatePicker.tsx
  • differenceInCalendarDays is used in RangeDatePicker.tsx and ProductSwitcher/helpers.ts
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usage of each imported date-fns function
rg "isAfter\(" 
rg "isSameDay\("
rg "differenceInCalendarDays\("

Length of output: 1042


220-220: Verify react-day-picker v9 API compliance

The API changes look correct but let's verify:

  • selectedDays object format
  • mode: 'range' setting
  • endMonth prop usage

Also applies to: 237-243

✅ Verification successful

API changes correctly implemented

The codebase confirms:

  • mode: 'range' is properly set in RangeDatePicker
  • selectedDays uses correct object format { from, to }
  • endMonth prop is consistently used across components, replacing toMonth
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for react-day-picker usage patterns
ast-grep --pattern 'mode: "range"'
ast-grep --pattern 'endMonth:'

Length of output: 67


Script:

#!/bin/bash
# Search for react-day-picker usage with more flexible patterns
rg -U "mode.*range" --type ts
rg "endMonth" --type ts
rg "selectedDays" --type ts -A 2

Length of output: 2261


246-246: Verify onSelect prop types

Ensure the onSelect callback type in IRangeDatePickerProps matches the handler implementation.

✅ Verification successful

Let me search for the handler implementation to verify the type matching.


Types are correctly matched

The onSelect prop type (selected: DateRange | null) => void aligns perfectly with the handler implementation that passes {from, to} object when both dates are selected.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for type definitions
rg "IRangeDatePickerProps" -A 10

Length of output: 11614


Script:

#!/bin/bash
# Search for handleOnSelect implementation
ast-grep --pattern 'const handleOnSelect = $_'

Length of output: 997

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file feature New feature or request
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants