DateTimePicker - invoke value change only on real change #3227
Conversation
nitzanyiz
changed the title
| const someDate = new Date('2021-04-04T00:00:00Z'); | ||
| const someDateNextDay = new Date(someDate.getTime() + 24 * 60 * 60 * 1000); | ||
| const someDateNextHour = new Date(someDate.getTime() + 60 * 60 * 1000); |
There was a problem hiding this comment.
Maybe:
sameTime \ sameDate
nextDay
nextHour
There was a problem hiding this comment.
Wdym here?
Do you think date, nextHour, nextDay is better?
There was a problem hiding this comment.
Yes, sorry I was not clear - this is about naming.
Personally I think sameTime, nextDay and nextHour.
There was a problem hiding this comment.
Why would you call it sameDate? What is it similar to?
I feel like its pretty clear, it also states that the date its self is not important.
There was a problem hiding this comment.
Because it's the same date as the date of the value, it's not a random "some date" it's the same date and the next day after the value and so on.
There was a problem hiding this comment.
I moved the date objects to the top of the file and used them in the default props.
| const someDate = new Date('2021-04-04T00:00:00Z'); | ||
| const someDateNextDay = new Date(someDate.getTime() + 24 * 60 * 60 * 1000); | ||
| const someDateNextHour = new Date(someDate.getTime() + 60 * 60 * 1000); | ||
| jest.mock('../../../optionalDependencies', () => { |
There was a problem hiding this comment.
Shouldn't this be in jest-setup.js?
There was a problem hiding this comment.
Not sure. jest-setup will mock this for every test file we have. I only see external modules in the jest-mock file i think its more organized this way.
Adding it to the jest-setup file also works. I guess we could do that. Do you think it will be clearer to put it there?
There was a problem hiding this comment.
Yes, I think that's the proper place for it because if we'll have an issue in private we'll look there first for a mock.
There was a problem hiding this comment.
What kind of issue will possibly have? When I you the jest mock here I don't think it has any effect on other test files. If I put it in the jest-setup one it does.
There was a problem hiding this comment.
If we'll need to mock the DateTimePickerPackage we'll probably look there and not here
M-i-k-e-l
deleted the
fix/DateTimePicker-invoke-value-change-on-real-change
branch
Description
Added handling to invoke onChange only when the value really change.
Changelog
DateTimePicker - onChange fixes. Only calls onChange when value really changes.
Additional info
MADS-4209
MADS-4210
MADS-4211