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

Ctrl/Cmd+Enter not working with date parameters #4025

Open
arikfr opened this issue Jul 31, 2019 · 9 comments
Open

Ctrl/Cmd+Enter not working with date parameters #4025

arikfr opened this issue Jul 31, 2019 · 9 comments
Assignees

Comments

@arikfr
Copy link
Member

arikfr commented Jul 31, 2019

Steps to Reproduce

  1. Change the value of a date or date range parameter.
  2. Hit Ctrl+Enter.

Expected: it would apply changes.
Actual: nothing happens. 😢

Technical details:

  • Redash Version: latest master
@ranbena
Copy link
Contributor

ranbena commented Jul 31, 2019

Looks like the calendar hijacks the keyboard, so ctrl+ent closes it then another applies.
I'll look into it.

@ranbena ranbena self-assigned this Jul 31, 2019
@ranbena
Copy link
Contributor

ranbena commented Jul 31, 2019

Oh I think it's just a matter of disabling the need for "ok" click as now it's not needed.

@gabrieldutra
Copy link
Member

gabrieldutra commented Aug 1, 2019

The behavior seems a bit different with me, am I missing something?

ctrl-enter-date-params
(Ctrl+enter applies the value and opens the calendar again as enter key press event is passed to Antd DatePicker)

@ranbena for the React version it's basically the same, with the difference that e.preventDefault() doesn't allow it to trigger the calendar opening.

I tested when the Calendar was closed, when it's open "the calendar hijacks the keyboard" seems to happen. The same doesn't happen in #4006.

@gabrieldutra
Copy link
Member

It is indeed not working when you select a Dynamic value to it, but in this case seems to be a matter of giving focus to the parameter after the selection.

@ranbena
Copy link
Contributor

ranbena commented Aug 1, 2019

It is indeed not working when you select a Dynamic value to it, but in this case seems to be a matter of giving focus to the parameter after the selection.

That's right.

I tested when the Calendar was closed, when it's open "the calendar hijacks the keyboard" seems to happen. The same doesn't happen in #4006.

Also right. Still, the calendar opens up on apply with ctrl+enter.
I struggled with it a bit but didn't get anywhere.

@gabrieldutra
Copy link
Member

Still, the calendar opens up on apply with ctrl+enter.

It doesn't on the React version 😝.

I'll start by working on the focus on the Date Picker and then we see where we can get :)

@ranbena
Copy link
Contributor

ranbena commented Aug 1, 2019

Still, the calendar opens up on apply with ctrl+enter.

It doesn't on the React version 😝.

I meant this: "(Ctrl+enter applies the value and opens the calendar again as enter key press event is passed to Antd DatePicker)"

@gabrieldutra
Copy link
Member

gabrieldutra commented Aug 1, 2019

I meant this: "(Ctrl+enter applies the value and opens the calendar again as enter key press event is passed to Antd DatePicker)"

Me too, the preventDefault worked to me it actually did not 😅.

@gabrieldutra
Copy link
Member

Not so related to the issue, #4033 will improve a little bit the behavior when you select dynamic values. Despite that, as I mentioned in the PR, Antd doesn't support key events in DatePicker api for now. Seems to be in their plans to extend it, but no idea on when (ref).

I believe it's possible to workaround that by forcing the focus to be somewhere else when the Calendar is closed, but I don't think it's worth it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants