Recomputes more modifiers when focusedInput changes #522
Conversation
Previously, only `props.isDayBlocked()` and `props.isDayHighlighted()` are computed. However, in my use case, where there are different validations on the start and end date inputs, `'blocked'` and `'blocked-out-of-range'` persisted when the focused input changes. This PR adds recomputation of modifiers with `this.isBlocked()` and `props.isOutOfRange()`. I've also added tests; however, it seems like `this.isBlocked()` is getting called for all visible dates instead of only the changed dates. I commented out the failing tests for now (returns 183 calls or something) -- more clarification on that would be appreciated! Also, if there's any reason I missed that these checks shouldn't be done, please let me know. I tested my fork in my use case and the problem was fixed. Thanks!
|
This PR is meant to fix #517. |
|
Woo! Thanks for adding this! I will look over this tonight or tomorrow! |
| @@ -796,6 +796,140 @@ describe('DayPickerRangeController', () => { | |||
| }); | |||
| }); | |||
|
|
|||
| describe('blocked', () => { | |||
| describe.skip('focusedInput did not change', () => { | |||
There was a problem hiding this comment.
This is one of the two tests I referenced in my commit message. I wasn't sure if it was appropriate to leave them out, or if it getting called 183 times actually isn't expected.
There was a problem hiding this comment.
So you probably have to reset the stub after the initial shallow render. The first mount is going to call this method a bunch of times! You want to isolate it to make sure that you're only checking how many times componentWillReceiveProps is calling it!
| }); | ||
| const blockedCalendarCalls = getCallsByModifier(addModifierSpy, 'blocked'); | ||
| expect(blockedCalendarCalls.length).to.equal(numVisibleDays); | ||
| isBlockedStub.restore(); |
There was a problem hiding this comment.
@majapw thanks for the tip re: resetting stubs! However, in this test and the following one, I need to make a stub that returns true or false. However, I don't ever need to access the stub, as blockedCalendarCalls is what's used to verify the test.
I realized I was mistakenly passing the stub in with the props in my previous commit (doing nothing) but after removing it I started getting a no-unused-vars lint error. So I shoehorned isBlockedStub.restore() in. Would this be an acceptable place for // eslint-ignore-line no-unused-vars?
Thanks!
There was a problem hiding this comment.
You don't need the restore - just don't assign the stub to a variable. In other words, sinon.stub(foo, bar); is fine by itself.
There was a problem hiding this comment.
Thank you, fixed
|
Awesome, thanks! |
|
Any idea when this will be released? I ran into this issue today. |
timhwang21
deleted the
timhwang21/recompute-modifiers-after-focusedinput-change
branch
Previously, only
props.isDayBlocked()andprops.isDayHighlighted()are computed. However, in my use case, where there are different validations on the start and end date inputs,'blocked'and'blocked-out-of-range'persisted when the focused input changes. This PR adds recomputation of modifiers withthis.isBlocked()andprops.isOutOfRange().I've also added tests; however, it seems like
this.isBlocked()is getting called for all visible dates instead of only the changed dates. I commented out the failing tests for now (returns 183 calls or something) -- more clarification on that would be appreciated!Also, if there's any reason I missed that these checks shouldn't be done, please let me know. I tested my fork in my use case and the problem was fixed.
Thanks!