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

Hide overlay on overlayblur #871

Conversation

matthova
Copy link

This PR addresses the bug described in issue #819.

Old behavior: When using a custom overlay, after clicking on anything in the overlay, then clicking away, the overlay does not close. Once the overlay has been focused, the only way to make the overlay close is to click on the input and then click away.

New behavior: After clicking on anything in the overlay, then clicking away, the overlay will close.

Note: With this PR - If the user clicks on the input field, after clicking on something inside of the overlay, the overlay will momentarily close and then reopen. This is not ideal, but it is better than the current behavior. If there is a preferred way to avoid calling setState when clicking on the input field, let's do that instead 👍.

🚀 Amazing library 🚀 I hope this PR helps

@codecov
Copy link

codecov bot commented Feb 22, 2019

Codecov Report

Merging #871 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #871   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          15     15           
  Lines         659    660    +1     
  Branches      146    146           
=====================================
+ Hits          659    660    +1
Impacted Files Coverage Δ
src/DayPickerInput.js 100% <100%> (ø) ⬆️

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 b086617...4d9698a. Read the comment docs.

2 similar comments
@codecov
Copy link

codecov bot commented Feb 22, 2019

Codecov Report

Merging #871 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #871   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          15     15           
  Lines         659    660    +1     
  Branches      146    146           
=====================================
+ Hits          659    660    +1
Impacted Files Coverage Δ
src/DayPickerInput.js 100% <100%> (ø) ⬆️

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 b086617...4d9698a. Read the comment docs.

@codecov
Copy link

codecov bot commented Feb 22, 2019

Codecov Report

Merging #871 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #871   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          15     15           
  Lines         659    660    +1     
  Branches      146    146           
=====================================
+ Hits          659    660    +1
Impacted Files Coverage Δ
src/DayPickerInput.js 100% <100%> (ø) ⬆️

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 b086617...4d9698a. Read the comment docs.

@matthova
Copy link
Author

Tested this a bit more. It solves one particular issue, but creates a bunch more issues with the standard React-Day-Input. Will revisit this once I have more time, but please let me know if a fix comes to mind.

@matthova
Copy link
Author

I'm not sure how to patch the bug while maintaining standard behavior.
Instead of pursuing fixing the bug as described in #819 I'm going to create a fork where showOverlay is driven by props.

@matthova matthova closed this Feb 25, 2019
@nevace
Copy link

nevace commented Mar 22, 2019

@matthova I had this same issue a while ago, stumbled across this while looking for a way to keep the overlay always open.

I ended up doing this to get around it, not ideal but might help

handleClickOutside = ({ target }) => {
    //this.datepicker is a ref to the container DayPickerInput is in and this.dayPickerInstance is the ref to DayPickerInput
    this.datepicker && !this.datepicker.contains(target) && this.dayPickerInstance.hideDayPicker();
};

componentDidMount() {
    document.addEventListener('click', this.handleClickOutside, true);
}

componentWillUnmount() {
    document.removeEventListener('click', this.handleClickOutside, true);
}

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.

None yet

2 participants