quasarframework / quasar

Quasar Framework - Build high-performance VueJS user interfaces in record time
https://quasar.dev
MIT License
25.77k stars 3.5k forks source link

Difficulties handling custom keyboard navigation with QSelect #12395

Open TuringJest opened 2 years ago

TuringJest commented 2 years ago

What happened?

I'm using QSelect in conjunction with another library and additional handling for keyboard navigation. Right now it is very difficult to implement advanced use-cases.

See Codesandbox for demonstration.

My main problems are:

  1. opening the popup immediately on focus. If I use @focus to show the popup it will also open @clear, as clear will trigger @focus. Also, it will mess with the popup when clicking on the select.
  2. Focusing another component with an open popup may leave the popup hanging (open the select popup in the sandbox and press 'n' on your keyboard)

What did you expect to happen?

It would be useful to have an event that fires when QSelect is focused while it was completely unfocussed before (no component within with focus etc.). It shouldn't mess with the popup behavior when clicking on it, however.

Also, very helpful events to have would be after a selection in the popup was made. @update will only fire if the option is different from the current one and @popup-hide will fire in many other scenarios. So it is more difficult if we want to close the popup immediately.

And please, please add a prop for the tabindex of the clear button. It is overcomplicating keyboard navigation in business applications.

The topic is quite complex as there are so many situations to consider and also might affect QSelect with an input etc. or date popup.

Reproduction URL

Codesandbox

How to reproduce?

show-popup interferes with click on select

  1. click on select

popup doesn't always close

  1. click on select and open the popup
  2. press 'n'

clearing opens popup because of show-popup in @focus handler

  1. select an option
  2. click on clear

Flavour

Quasar CLI (@quasar/cli | @quasar/app)

Areas

Components (quasar)

Platforms/Browsers

No response

Quasar info output

Operating System - Darwin(21.3.0) - darwin/arm64
NodeJs - 16.6.1

Global packages
  NPM - 8.3.0
  yarn - 1.22.11
  @quasar/cli - 1.2.2
  @quasar/icongenie - Not installed
  cordova - Not installed

Important local packages
  quasar - 2.5.3 -- Build high-performance VueJS user interfaces (SPA, PWA, SSR, Mobile and Desktop) in record time
  @quasar/app - 3.3.2 -- Quasar Framework local CLI
  @quasar/extras - 1.12.4 -- Quasar Framework fonts, icons and animations
  eslint-plugin-quasar - Not installed
  vue - 3.2.29 -- The progressive JavaScript framework for building modern web UI.
  vue-router - 4.0.12
  vuex - 4.0.2 -- state management for Vue.js
  electron - 17.0.0 -- Build cross platform desktop apps with JavaScript, HTML, and CSS
  electron-packager - 15.4.0 -- Customize and package your Electron app with OS-specific bundles (.app, .exe, etc.) via JS or CLI
  electron-builder - Not installed
  @babel/core - 7.14.3 -- Babel compiler core.
  webpack - 5.64.1 -- Packs CommonJs/AMD modules for the browser. Allows to split your codebase into multiple bundles, which can be loaded on demand. Support loaders to preprocess files, i.e. json, jsx, es7, css, less, ... and your custom stuff.
  webpack-dev-server - 4.7.3 -- Serves a webpack app. Updates the browser on changes.
  workbox-webpack-plugin - Not installed
  register-service-worker - 1.7.2 -- Script for registering service worker, with hooks
  typescript - 4.5.5 -- TypeScript is a language for application scale JavaScript development
  @capacitor/core - Not installed
  @capacitor/cli - Not installed
  @capacitor/android - Not installed
  @capacitor/ios - Not installed

Quasar App Extensions
  @quasar/quasar-app-extension-dotenv - 1.1.0 -- Load .env variables into your quasar project

Networking
  Host - MacBook-Pro.local
  en0 - 192.168.20.5

Relevant log output

No response

Additional context

No response

github-actions[bot] commented 2 years ago

Hi @TuringJest! 👋

It looks like you provided an invalid or unsupported reproduction URL. Do not use any service other than Codepen, jsFiddle, Codesandbox, and GitHub. Make sure the URL you provided is correct and reachable. You can test it by visiting it in a private tab, another device, etc. Please edit your original post above and provide a valid reproduction URL as explained.

Without a proper reproduction, your issue will have to get closed.

Thank you for your collaboration. 👏

TuringJest commented 2 years ago

@rstoenescu any ideas on how we can improve this?

We still have issues figuring out a way to open the popup immediately on focus without getting issues on click:

  1. on focus opens the popup, then on click will toggle back to close
  2. also hiding the popup will trigger on focus opening the popup again. So now the popup opens every time I clear.

I use QSelect inside a table (AG-Grid) and the navigation is mostly hardcoded there (AG-Grid will look for an input element and focus it). I can not tell AG-Grid to call showPopup when focusing the QSelect.

Also I have trouble doing something on keydown.enter on the popup which doesn't collide with keydown.enter on the QSelect. We 'exit' the QSelect after an option was selected (keydown.enter or click) you can check the codesandbox to understand why. Right now I can do this on update which doesn't work if the selection doesn't update or on popup-hide which also triggers in completely different cases. Not ideal.

I think as a first step it would help if we can make a v-model for the popup state, so we have a way to distinguish between keydown events on the popup and events on the QSelect. An event like on popup-keydown-enter might also help. As the popup is not rendered inside the QSelect, its more difficult to manually check the DOM or bind native listeners.

Overall I think if we want QSelect to behave nicely with custom navigation and more complex use cases, we have to be able to tackle those issues. For some apps every additional click and interaction needs to be justified.

ramicohen303 commented 2 years ago

Might be related, but tabbing into QSelect does not show the popup, while mouse-clicking the QSelect does show it. This is inconsistent behavior, the popup should show, no matter how the control got the focus.

pdanpdan commented 2 years ago

@ramicohen303 there is nothing inconsistent about that. Click is not focus, click is focus and enter or space.

TuringJest commented 2 years ago

@pdanpdan it can lead to annoyances with keyboard navigation. Did you have time to look at the sandbox? I could not solve the issue. Now the table cells with a q-select without an input need one enter event to focus the q-select and another one to open the popup. Every other cell takes one enter to input something or make a selection. On the other hand it takes one click in both cases to do the same.

I guess there is always a chance you can end up with a library where control is limited. @ramicohen303 has a good point, if focus and click are treated the same those issues would be non-existent.

pdanpdan commented 2 years ago

I changed it so it works: https://codesandbox.io/s/winter-wind-mnbhwk?file=/src/components/Wrapper.vue

TuringJest commented 2 years ago

@pdanpdan thank you for looking into it.

Unfortunately, this is not possible because the wrapper where you added the code would be a library/ 3rd party package (Ag-Grid) component in this case, so we can't modify the wrapper (hence the comment on the top of the component).

I still don't see how we can solve this with the q-select the way it works now.
Might only affect a few quasar users but it is a big headache.

TuringJest commented 2 years ago

Why? AG-Grid is table component they render all the cells and add navigation etc., the cells are like the wrapper in the sandbox. We can tell Ag-Grid to render our custom components (e.g. q-select) inside the cell and it will then query for a focusable element inside the custom comp and focus it when we want to interact with it (e.g. edit...). Can't add our logic to that. They do supply event handlers for some cells but not for header cells so there is no way to intercept the 'enter' event and handle it our way either.

I can spin up a codesandbox for an AG-Grid example, just didn't want to make the example bigger than necessary.

One more thing: in your example there is still the issue that we can't easily focus back to the wrapper with enter when the popup is open and the model value will not change (if 1. same option as selected one or 2. no option is selected).

We can't easily know if the popup is opened so that's problematic as it also triggers the popup. Popup is rendered through a portal so a v-model for the popup state would be nice.

TuringJest commented 2 years ago

So far I see only one way to do it, have an invisible input together with the q-select which acts as a target for the query and focus of the grid and there call methods to open the q-select. But that's some MacGyver hacking I want to avoid.

Edit: split into comments for better readability, sorry for many edits :))

pdanpdan commented 2 years ago

Updated pen to also listen for @popup-hide="wrapper1?.$el?.focus()" I think you should use a simplified (without tab navigation, ...?) version of the wrapper in the pen to what your QSelect in AgGrid cell, maybe it works

If you can make a pen with a very simple AgGrid it would help

TuringJest commented 2 years ago

@pdanpdan codesandbox kept freezing, here is a github repo https://github.com/TuringJest/quasar_aggrid

There are two QSelect customComponents which are used in the header (AgGrid floatingHeader for filtering) only. We also use them in table cells, but the headers combine all the QSelect edge cases.

One component uses the idea of sibling inputs to intercept the focus from Ag-Grid... it was very close to what we tried to achieve, but I decided to not follow it because it will create new issues with a11y, required reimplementing AG-Grids tab navigation, and is ugly. There is another leaner version, which is similar to what we currently use for the header, but it's still lacking.