Skip to content

Conversation

morenyang
Copy link
Contributor

Refs to ant-design/ant-design#21084

Update viewDates when any item is selected.
Whether this will cause other impacts, needs further review.

@vercel
Copy link

vercel bot commented Jan 26, 2020

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/react-component/picker/ky9mbsvia
✅ Preview: https://picker-git-fork-morenyang-range-picker-date-issue.react-component.now.sh

@codecov
Copy link

codecov bot commented Jan 26, 2020

Codecov Report

Merging #21 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #21      +/-   ##
==========================================
- Coverage   99.55%   99.55%   -0.01%     
==========================================
  Files          43       43              
  Lines        1793     1790       -3     
  Branches      525      534       +9     
==========================================
- Hits         1785     1782       -3     
  Misses          6        6              
  Partials        2        2              
Impacted Files Coverage Δ
src/hooks/useRangeViewDates.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8c13ab7...2cd6694. Read the comment docs.

@hannaofsweden
Copy link

Please fix this one :)

wrapper.openPicker(1);
wrapper.selectCell(1993);
expect(isSame(onPanelChange.mock.calls[0][0][1], '1993-09-03'));
expect(isSame(onPanelChange.mock.calls[0][0][1], '1993-02-03'));
Copy link
Member

Choose a reason for hiding this comment

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

Should fix logic instead of change test case.

@hannaofsweden
Copy link

Any progress on this one @morenyang ?

@morenyang
Copy link
Contributor Author

Hi, @zombieJ , I reviewed the issue description and code again,

in the original comment, it says to reset another one when empty, but the condition seems to be wrong

// Reset another one when not have value
const anotherIndex = (index + 1) % 2;
if (getValue(values, anotherIndex)) {
newViewDates = updateValues(newViewDates, viewDate, anotherIndex);
}

so I reverse the condition and it works

// Reset another one when not have value
const anotherIndex = (index + 1) % 2;
if (!getValue(values, anotherIndex)) {
newViewDates = updateValues(newViewDates, viewDate, anotherIndex);
}

@morenyang morenyang changed the title fix: ant-design/ant-design#21084 range picker date issue fix: ant-design/ant-design#21084 improve range picker calendar display on select Mar 21, 2020
@zombieJ
Copy link
Member

zombieJ commented Mar 24, 2020

Got it. You are correct.
Could you help to add a test case for this?

@morenyang
Copy link
Contributor Author

Got it. You are correct.
Could you help to add a test case for this?

Okay, I may add some test cases when I have time

@morenyang morenyang force-pushed the range-picker-date-issue branch from 7f7e956 to 13aa6a3 Compare May 19, 2020 13:59
@morenyang morenyang force-pushed the range-picker-date-issue branch from 13aa6a3 to 53bc14d Compare May 19, 2020 14:05
@morenyang morenyang requested a review from zombieJ May 19, 2020 14:13
@morenyang
Copy link
Contributor Author

Hi @zombieJ , add a test case for this

@zombieJ zombieJ merged commit 5a54895 into react-component:master May 20, 2020
@zombieJ
Copy link
Member

zombieJ commented May 20, 2020

+ rc-picker@1.5.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants