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

Added support for "disableOnClickOutside" property. #405

Merged
merged 2 commits into from
Feb 8, 2018

Conversation

dennis90
Copy link
Contributor

Added functionality to do not close calendar when disableOnClickOutside is set to true.

Description

Motivation and Context

This pull request fixes bug #402, adding support for disableOnClickOutside documented property.

Checklist

[X] I have added tests covering my changes
[X] All new and existing tests pass
[ ] My changes required the documentation to be updated
  [ ] I have updated the documentation accordingly
  [ ] I have updated the TypeScript 1.8 type definitions accordingly
  [ ] I have updated the TypeScript 2.0+ type definitions accordingly

@dennis90 dennis90 closed this Feb 6, 2018
@simeg simeg reopened this Feb 7, 2018
@simeg
Copy link
Collaborator

simeg commented Feb 7, 2018

Hi @dennis90. I re-opened this PR because I like it. However, when I merge your changes with master one of the tests you wrote fails.

FAIL  test/tests.spec.js (6.04s)
  ● Datetime › with custom props › disableOnClickOutside=false

    expect(received).toBeFalsy()

    Expected value to be falsy, instead received
      true

I'm not sure why it's failing, if you want to you can investigate.

@dennis90
Copy link
Contributor Author

dennis90 commented Feb 7, 2018

@simeg thanks for your update,
I fixed the failing tests, you can see what was going on:

enzymejs/enzyme#1229

@simeg
Copy link
Collaborator

simeg commented Feb 8, 2018

Cool, thanks! I'll remember to keep that component.update() in mind.

@simeg simeg merged commit 06f14d1 into arqex:master Feb 8, 2018
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.

2 participants