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

Remove disableTouchKeyboard and forced focus #78

Closed
lubomirblazekcz opened this issue Sep 30, 2021 · 2 comments
Closed

Remove disableTouchKeyboard and forced focus #78

lubomirblazekcz opened this issue Sep 30, 2021 · 2 comments

Comments

@lubomirblazekcz
Copy link

if (!datepicker.inline && !datepicker.config.disableTouchKeyboard) {

This line prevents me from extend the datepicker with timepicker for example.
obrazek

Because everytime I click into the time input, the focus is lost. I'm not even sure what's the purpose of that focus function.

Anyway, I think disabling touch keyboard should be done with inputmode="none", it's a better approach

@mymth
Copy link
Owner

mymth commented Oct 4, 2021

In order for keyboard operation to work, the input field has to be focused. If the focus is not moved back to the input field there, it goes to <body> and no keydown event will be fired on the input field until you manually move the focus back. You'll be unable to move to the adjacent field by tab key press too.

Also, to prevent the input filed from losing focus by other than user's intention, that listener function is added to the capture phase of the input field's click event so that it runs before all other on-click stuff is executed. (#21, #47)
For this reason, if you want the stuff you injected to the date picker to get the focus, you need to patch the function to be something like this:

export function onClickPicker(datepicker, ev) {
  if (ev.target.matches('.timepicker') {
    // do the timepicker stuff
    // ...
  } else if (!datepicker.inline && !datepicker.config.disableTouchKeyboard) {
    datepicker.inputField.focus();
  }
};

You might see the date picker behave unexpectedly if you don't move the focus back to the input field appropriately after time picking is done.

As for disableTouchKeyboard, (I might remember something wrong because my memory is vague now...) I considered rewriting it with inputmode="none" and concluded that, since it's an <input> element's attribute, it should be coded in user's HTML and should not be overridden by the program. And just in case someone wants it for backward compatibility, I decided to leave it so that it to work in the same way as bootstrap-datepicker.
But now I'm thinking about removing the option because I noticed it no longer works as expected on my iPhone.

@lubomirblazekcz
Copy link
Author

I see, I did not realize it's there for tabulator and overall accessbility. Would you consider editing the function with the option for ignored elements maybe? Or maybe the focus should be triggered only on datepicker action elements as .datepicker-cell and .button?

Extending the picker with timepicker or anything other than that would be easier after that, and I can always trigger the focus back to input manually after that.

And yeah using inputmode="none" should be definetly on the user, since it's native attribute. I think disableTouchKeyboard is kind of redundant option now. But I would leave info how to disable mobile keyboard in docs, not all people know about inputmode, for some it's still something new :)

mymth added a commit that referenced this issue Dec 29, 2021
…ng default of mousedown event in order to prevent input field from flickering and give users a way to extend datepicker by adding their own elements (#78)

Close #85, Close #92
@mymth mymth closed this as completed in fc5ba5a Dec 29, 2021
mymth added a commit that referenced this issue Dec 29, 2021
…ng default of mousedown event in order to prevent input field from flickering and give users a way to extend datepicker by adding their own elements (#78)

Close #85, Close #92

* Merge branch 'dev'
mymth added a commit that referenced this issue Dec 29, 2021
Close #78

* Merge branch 'dev'
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 a pull request may close this issue.

2 participants