mymth / vanillajs-datepicker

A vanilla JavaScript remake of bootstrap-datepicker for Bulma and other CSS frameworks
MIT License
720 stars 147 forks source link

Prevent opening datepicker on Escape #103

Closed ragulka closed 1 year ago

ragulka commented 2 years ago

Pressing Escape on the input field will open the datepicker. I find this rather odd, and unnecessary. Consider that the picker field may be inside a modal, which will be closed on Escape. When Escape is pressed, it would momentarily flash open the datepicker inside the modal, right before the modal is hidden. Would you consider either removing this behavior or providing an option to toggle it?

mymth commented 2 years ago

I would like you to go to https://uxsolutions.github.io/bootstrap-datepicker and check what happens if you open the date picker then repeatedly press the escape key 2 or 4 times. Maybe you don't know what it is for because you have only used the date picker inside a modal dialog. If so, you’ll be able to find what it’s meant for. And if not, you’ll be able to notice it’s an inheritance from bootstrap-datepicker.

Since it’s a feature that has been around for years, I believe there are people who use this feature. Therefore, I will not consider removing it at least until the timing for major version update

If datepicker shows up by pressing Escape key, it means the picker is hidden while the input field has the focus. The 2 most common cases are after selecting a date when the autohide option is on (see https://github.com/mymth/vanillajs-datepicker/issues/47#issuecomment-766495408) and after user manually hides the picker by pressing Escape. But we can ignore the latter now because it doesn’t happen when datepicker is in a modal.

When autohide is off, which is default, and showOnFocus is on, which is also default, picker's visibility is synced with the input field’s active status. If Escape is pressed while the focus is on date input (the picker is shown), both the picker and the modal close at once because the event is fired on the date input then bubbles up to the modal. And if Escape is pressed while the focus is on other element than date input (the picker is hidden), the modal closes without showing the picker as the event is fired on the other element then bubbles up to the modal. For this reason, the issue you mentioned doesn’t happen with the default settings.

If you want to use autohide, and prevent the flickering as well, it can be done by unfocusing the input field with moving the focus to other field in the modal or the modal itself after autohiding occurs, like in this example: https://codepen.io/mymth/pen/wvpGePw

So, since the issue is not critical, only occurs under a specific condition, and can be avoided quite easily, I would not consider adding the option to disable the feature either.

ragulka commented 2 years ago

@mymth thanks for the detailed description! I can see how this would be a breaking change, and I can definitely understand the background for the current behavior. The proposed workaround for the modal might help with the flicker, sure. However, there are other factors at play, as well.

You mention in https://github.com/mymth/vanillajs-datepicker/issues/47#issuecomment-766495408 that you try "to make the date picker behave as close to native elements as possible and consistently regardless of being operated by mouse or keyboard is one of the key concepts of this library". I wholeheartedly agree with this concept, which is why I'd like to elaborate on my case.

My goal is to create a datepicker which behaves very similar to a native select or date input, and use it inside a modal. Consider the examples in this forked pen: https://codepen.io/ragulka/pen/RwxRJeg

Native select and input type="date":

☝️ I'm trying to replicate the above behavior

Datepicker with default settings:

Datepicker with tweaks: (picker dropdown will open when clicking or typing in the date field)

As you can see, I can get somewhat close to the native control behavior using showOnFocus: false combined with autohide: true. To achieve the last 2 criteria, the following would be needed:

If the goal is make a library that behaves similar to native form controls, wouldn't it make sense to at least provide options for doing so?

I'd be happy to submit a PR if you think you can agree with the general direction.

mymth commented 2 years ago

OK, I get you point. (Why you didn’t say that from the beginning?) However, I’m reluctant to add patchy option without seeing high demand. If you could submit a PR to add an option to change the key to toggle picker’s visibility, it would be more preferable and welcome. (This is to prevent the code from becoming spaghetti like bootstrap-datepicker. It would be appreciated if you understand the policy.)

mymth commented 2 years ago

By the way, the bubbling of Escape keydown event is actually a bug from the changes for #88. So it will be fixed in the next version.