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

fix(date-picke): close range datepicker after clicking elsewhere on page #851

Merged
merged 4 commits into from
Jun 18, 2018

Conversation

rezak-otmani
Copy link
Contributor

@rezak-otmani rezak-otmani commented Jun 4, 2018

Overview

Resolves #772

Close the date picker icon when this component loses focus

Added

Removed

Changed

Testing / Reviewing

Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

Thanks @rezak-otmani for trying to address the problem. Could you try clicking on start date and then clicking on end date, and ensure that the date picker is still shown? The code you tried to remove has a reason being there as the comment says, and want to ensure that the purpose of the code won't be broken. Thanks!

@rezak-otmani
Copy link
Contributor Author

Hi @asudoh, I don't know exactly if there is a side effect of the code I deleted,, but when we click on the start date, and then on the end date the date picker is always displayed, and it works

Copy link
Member

@alisonjoseph alisonjoseph left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@asudoh I double checked the config in flatpicker and its related to having it always open, which I don't believe is the intended behavior. So this fix should be ok. https://flatpickr.js.org/examples/#inline-calendar

@asudoh
Copy link
Contributor

asudoh commented Jun 5, 2018

Thanks @rezak-otmani and @alisonjoseph for diving into this topic! Here’s what I see with @rezak-otmani’s change:

without-workaround

And here’s what I expect:

with-workaround

@alisonjoseph is right that the usage of that option is not something Flatpickr intended, but the code was to prevent date picker from being closed in the scenario I brought up. Flatpickr has the code to close the dropdown in that scenario, and cannot be disabled unless we temporarily set inline option.

Hope it makes sense to everybody. Please don’t hesitate to bring up if there are any questions. Thanks!

Copy link
Member

@alisonjoseph alisonjoseph left a comment

Choose a reason for hiding this comment

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

I see what @asudoh is saying. So while this solves the issue of being able to click outside the dropdown after clicking on the calendar icon to close it, it creates a new issue where you can't click between input boxes without closing the dropdown prematurely.

Copy link
Contributor

@marijohannessen marijohannessen left a comment

Choose a reason for hiding this comment

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

It's working for me! 👍 ✅

@marijohannessen
Copy link
Contributor

@alisonjoseph can you review again and see if you're still seeing the issue?

@alisonjoseph
Copy link
Member

LGTM now! ✅

@alisonjoseph alisonjoseph merged commit c978c4f into carbon-design-system:master Jun 18, 2018
@carbon-bot
Copy link
Contributor

🎉 This PR is included in version 9.0.9 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

6 participants