feat(DateRangePicker): limit start year #3151

loposkin · 2023-04-07T18:51:20Z

CHANGES

Added limitStartYear prop to DateRangePicker. Usage:

<DateRangePicker
          limitEndYear={1}
          limitStartYear={5}
          ...
>

This looks like:

If unset, the start year still defaults to 1900

ISSUE

close #3145

vercel · 2023-04-07T18:51:24Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
rsuite-nextjs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 17, 2023 2:23pm

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
rsuite-v4	⬜️ Ignored (Inspect)			Apr 17, 2023 2:23pm

vercel · 2023-04-07T18:51:25Z

@loposkin is attempting to deploy a commit to the rsuite Team on Vercel.

A member of the Team first needs to authorize it.

codesandbox-ci · 2023-04-07T18:53:09Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 9e9d1d6:

Sandbox	Source
rsuite-tp-ci	Configuration

codecov · 2023-04-10T06:45:39Z

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 🎉

Comparison is base (786e930) 93.43% compared to head (a184e82) 93.44%.

❗ Current head a184e82 differs from pull request most recent head 9e9d1d6. Consider uploading reports for the commit 9e9d1d6 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3151      +/-   ##
==========================================
+ Coverage   93.43%   93.44%   +0.01%     
==========================================
  Files         562      563       +1     
  Lines       20395    20415      +20     
  Branches     2739     2740       +1     
==========================================
+ Hits        19056    19077      +21     
  Misses        694      694              
+ Partials      645      644       -1

Flag	Coverage Δ
ChromeCi	`93.40% <100.00%> (+0.01%)`	⬆️
Firefox	`92.70% <100.00%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/Calendar/CalendarContainer.tsx	`100.00% <ø> (ø)`
src/DateRangePicker/Calendar.tsx	`96.55% <ø> (ø)`
src/Calendar/Calendar.tsx	`90.00% <100.00%> (+0.34%)`	⬆️
src/Calendar/MonthDropdown.tsx	`100.00% <100.00%> (+2.22%)`	⬆️
src/Calendar/test/CalendarMonthDropdownSpec.tsx	`100.00% <100.00%> (ø)`
src/DatePicker/DatePicker.tsx	`94.63% <100.00%> (ø)`
src/DateRangePicker/DateRangePicker.tsx	`88.03% <100.00%> (ø)`
...DateRangePicker/test/CalendarMonthDropdownSpec.tsx	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

loposkin · 2023-04-10T08:36:03Z

@simonguo, do I need to do something else? Looks like the authorization is missing on some test stage.

SevenOutman

Hi @loposkin, thank you for contributing this edit!

As this PR adds new APIs, please add test cases to ensure the API work as expected.

loposkin · 2023-04-11T07:31:31Z

@SevenOutman, I've added a test for a MonthDropdown's limitStartYear and limitEndYear props. I've also added missing propTypes.

SevenOutman · 2023-04-12T08:13:08Z

src/DateRangePicker/test/CalendarMonthDropdownSpec.tsx

+    expect(getAllByRole('row', { hidden: true })).to.be.lengthOf(2);
+    expect(getAllByRole('rowheader', { hidden: true })[0].innerText).to.be.eq(
+      previousYear.toString()
+    );
+    expect(getAllByRole('rowheader', { hidden: true })[1].innerText).to.be.eq(
+      currentYear.toString()
+    );


Let me confirm this. Does it mean that limitStartYear={1} results in the earliest year be 1 year before current, while limitEndYear={1} results in the latest year be just current year? If so, I don't think it's consistent.

Hi, yes, you're right and yes, it's not consistent. The limitStartYear={1} results in the earliest year that is 1 year before current, while limitEndYear={1} results in the current year. The problem is that I only propagate the props to existent MonthDropdown-API. Should I change this API? I think it would mean a breaking change for people who already use MonthDropdown.

simonguo · 2023-04-12T14:06:12Z

Hi @loposkin

If the limitEndYear and limitStartYear props are to be exposed, I hope he can make some adjustments in the design to make it more reasonable to use.

limitStartYear?: number | ((activeDate: Date) => number);
limitEndYear?: number | ((activeDate: Date) => number);

where the number value should be a year.

<DateRangePicker limitStartYear={1990} limitEndYear={2023} />
<DateRangePicker
  limitStartYear={(activeDate: Date) => {
    return addDays(getYear(activeDate), -5);
  }}
  limitEndYear={(activeDate: Date) => {
    return addDays(getYear(activeDate), 5);
  }}
/>;

Also limitEndYear and limitStartYear props should be supported on both DatePicker and Calendar.

…solute values

loposkin · 2023-04-12T15:39:45Z

@simonguo @SevenOutman , I've changed the API according to @simonguo suggestion. I've also updated Calendar's and DatePicker's API accordingly. limitStartYear and limitEndYear are now consistent and the api looks like:

<DateRangePicker
          limitEndYear={2018}
          limitStartYear={2023}
          ...
>

I've updated the english documentation accordingly, unfortunatelly, I am missing required language skills to update the documentation in Chinese.

As for functional values for limitStartYear and limitEndYear props, this exceeds the original issue and I have no resources to implement it right now.

SevenOutman · 2023-04-14T05:55:31Z

Hi, yes, you're right and yes, it's not consistent. The limitStartYear={1} results in the earliest year that is 1 year before current, while limitEndYear={1} results in the current year. The problem is that I only propagate the props to existent MonthDropdown-API. Should I change this API? I think it would mean a breaking change for people who already use MonthDropdown.

@loposkin, MonthDropdown is an internal component that's not exported as a public API, so I think it's OK to either change it's limitStartYear behavior, or simply leave it as is and change the value that DateRangePicker passes to MonthDropdown. That way limitStartYear of DateRangePicker (as a public API) is consistent with limitEndYear, and minimum changes have to be made.

If the limitEndYear and limitStartYear props are to be exposed, I hope he can make some adjustments in the design to make it more reasonable to use.
where the number value should be a year.
<DateRangePicker limitStartYear={1990} limitEndYear={2023} />

@simonguo, the limitEndYear prop already exists on DateRangePicker, so a direct change like this is breaking. I don't think it's appropriate to introduce this change in the current major release. I agree that limitStartYear and limitEndYear should have been more straightforward, we can make this change in next major release with migration guidance for users.

loposkin · 2023-04-14T06:16:50Z

@SevenOutman, so if I change the api to be consistent to existent limitEndYear APIs, such that <DateRangePicker limitStartYear={1} limitEndYear={1} /> will only show current year and <DateRangePicker limitStartYear={2} limitEndYear={2} /> will show the years from 2022 to 2024, would it be possible to merge this in the next release? Or would you rather implement it properly in the next major release?

SevenOutman · 2023-04-17T05:20:07Z

@SevenOutman, so if I change the api to be consistent to existent limitEndYear APIs, such that <DateRangePicker limitStartYear={1} limitEndYear={1} /> will only show current year and <DateRangePicker limitStartYear={2} limitEndYear={2} /> will show the years from 2022 to 2024, would it be possible to merge this in the next release? Or would you rather implement it properly in the next major release?

It's going to be merged.

loposkin · 2023-04-17T15:26:15Z

@SevenOutman, I've opened another PR with requested changes.

loposkin added 2 commits April 7, 2023 20:32

feat(DateRangePicker): added limit start year to DateRangePicker

cb4a8cd

feat(DateRangePicker): added docs, fixed property order

13ad20f

vercel bot deployed to Preview – rsuite-nextjs April 7, 2023 18:54 View deployment

SevenOutman requested a review from simonguo April 10, 2023 06:40

SevenOutman requested changes Apr 11, 2023

View reviewed changes

loposkin added 2 commits April 11, 2023 09:39

feat(DateRangePicker): added proptypes for limitStartYear

fe09bfa

feat(DateRangePicker): added test for limitStartYear and limitEndYear

3dc77cf

vercel bot deployed to Preview – rsuite-nextjs April 11, 2023 08:25 View deployment

loposkin requested a review from SevenOutman April 11, 2023 11:47

SevenOutman reviewed Apr 12, 2023

View reviewed changes

loposkin added 4 commits April 12, 2023 17:15

feat(DateRangePicker): changed limit start year and end year to be ab…

2bd5590

…solute values

feat(DateRangePicker): added limit props propagation to Calendar

adb0cd4

feat(DateRangePicker): reverted package.json changes

22d20ed

feat(DateRangePicker): fixed order of props in MonthDropdown

959f8ff

vercel bot deployed to Preview – rsuite-nextjs April 12, 2023 15:32 View deployment

feat(DateRangePicker): added additional test case

507aee5

vercel bot deployed to Preview – rsuite-nextjs April 12, 2023 15:39 View deployment

feat(DateRangePicker): fixed docs for date range picker

a184e82

vercel bot deployed to Preview – rsuite-nextjs April 12, 2023 15:45 View deployment

loposkin requested a review from SevenOutman April 12, 2023 18:02

Merge branch 'rsuite:main' into main

9e9d1d6

vercel bot deployed to Preview – rsuite-nextjs April 17, 2023 14:23 View deployment

loposkin closed this Apr 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(DateRangePicker): limit start year #3151

feat(DateRangePicker): limit start year #3151

loposkin commented Apr 7, 2023

vercel bot commented Apr 7, 2023 •

edited

Loading

vercel bot commented Apr 7, 2023

codesandbox-ci bot commented Apr 7, 2023 •

edited

Loading

codecov bot commented Apr 10, 2023 •

edited

Loading

loposkin commented Apr 10, 2023 •

edited

Loading

SevenOutman left a comment

loposkin commented Apr 11, 2023 •

edited

Loading

SevenOutman Apr 12, 2023 •

edited

Loading

loposkin Apr 12, 2023

simonguo commented Apr 12, 2023

loposkin commented Apr 12, 2023

SevenOutman commented Apr 14, 2023

loposkin commented Apr 14, 2023

SevenOutman commented Apr 17, 2023

loposkin commented Apr 17, 2023

feat(DateRangePicker): limit start year #3151

feat(DateRangePicker): limit start year #3151

Conversation

loposkin commented Apr 7, 2023

CHANGES

ISSUE

vercel bot commented Apr 7, 2023 • edited Loading

vercel bot commented Apr 7, 2023

codesandbox-ci bot commented Apr 7, 2023 • edited Loading

codecov bot commented Apr 10, 2023 • edited Loading

Codecov Report

loposkin commented Apr 10, 2023 • edited Loading

SevenOutman left a comment

Choose a reason for hiding this comment

loposkin commented Apr 11, 2023 • edited Loading

SevenOutman Apr 12, 2023 • edited Loading

Choose a reason for hiding this comment

loposkin Apr 12, 2023

Choose a reason for hiding this comment

simonguo commented Apr 12, 2023

loposkin commented Apr 12, 2023

SevenOutman commented Apr 14, 2023

loposkin commented Apr 14, 2023

SevenOutman commented Apr 17, 2023

loposkin commented Apr 17, 2023

vercel bot commented Apr 7, 2023 •

edited

Loading

codesandbox-ci bot commented Apr 7, 2023 •

edited

Loading

codecov bot commented Apr 10, 2023 •

edited

Loading

loposkin commented Apr 10, 2023 •

edited

Loading

loposkin commented Apr 11, 2023 •

edited

Loading

SevenOutman Apr 12, 2023 •

edited

Loading