search

getredash / redash

Make Your Company Data Driven. Connect to any data source, easily visualize, dashboard and share your data.
http://redash.io/
BSD 2-Clause "Simplified" License
26k stars 4.34k forks source link

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

Open arikfr opened 5 years ago

arikfr commented 5 years ago

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. :cry:

Technical details:

ranbena commented 5 years ago

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

ranbena commented 5 years ago

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

gabrieldutra commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

It doesn't on the React version :stuck_out_tongue_closed_eyes:.

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

ranbena commented 5 years ago

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 commented 5 years ago

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 :sweat_smile:.

gabrieldutra commented 5 years ago

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.