"Tab" Keyboard Event suppressed in modal windows

[josephjeno]

Hi there Anders, hope all is well and you are staying safe.

We noticed a bug today when attempting to "tab" within a modal dialog, the "tab" is squashed by OS.js as the target is not contained by the OS.js window.

src/desktop.js

        if (isWithinWindow(w, e.target)) {
          if (isInput) {
            if (tagName === 'TEXTAREA') {
              handleTabOnTextarea(e);
            }
          } else {
            e.preventDefault();
          }
        } else {
          e.preventDefault();
        }

    const isWithinWindow = (w, target) => {
      return w && w.$element.contains(target);
    };

w.$element

target

[valp124]

I’ll just add that this is a Material UI modal dialog, although that might be obvious. Thank you so much, Anders!

[andersevenrud]

I see. I think a possible solution would be to make it so osjs-client resets the internal reference on a window when blur() is called.

This way you can call blur() when you open the modal and focus() back when you close it. Does this sound like a good solution?

Another thing you could do is to put your modal inside an actual window. There's some mechanics of having actual modals, i.e. disable parent window etc.

[josephjeno]

The blur()/focus() solution sounds good to me!

[valp124]

Second that. (I hope we don't have too many places where we'll have to inject the blur()/focus() calls!)

[andersevenrud]

After thinking about this for a while longer, this might introduce some unwanted side effects.

The reason the window check is there is so that the tabbing is contained. Removing this probably makes it so you can tab outside your modal.

So this is another suggestion I have for a solution: Add an API method to set the current "root" element for the desktop that is prioritized higher than the window. Ex:

const {setKeyboardContext} = core.make('osjs/desktop');
setKeyboardContext(el); // el = root of the modal makes tab trigger there
setKeyboardContext(); // any falsy argument results in tab trigger on window

So, not much different in usage, just how it works on the bottom.

[valp124]

Just to confirm your suspicion, Anders: you're correct, removing the check enables tabbing, but then tabs go outside the modal if you keep hitting the key.

[andersevenrud]

I'm going for a workout, but I'll write a patch for my most recent suggestion when I get back :)

[andersevenrud]

I managed to pull it off quickly before I went out. Feel free to test it and let me know how it worked out :blush:

https://github.com/os-js/osjs-client/pull/103

[valp124]

Will do! Thank you so much!

[valp124]

No... very unfortunately, it didn't. I updated @osjs/client -- that's it, right? Still the same event handler is causing the problem...

[andersevenrud]

The patch is in PR #103 only and not on npm, so you'll have to set up a local @osjs/client with the branch over there to get it (hopefully) working.

[valp124]

Aha! I see. Pardon my ignorance - and how do we do the build in that case?

[andersevenrud]

Example here: https://manual.os-js.org/v3/development/#replacement

And to use the patch, follow command line instructions link in the PR.

[andersevenrud]

This is probably a faster way, because the PR instructions introduces a few extra steps...

In your distro:

cd src/
git clone https://github.com/os-js/osjs-client
cd osjs-client
git checkout feature/desktop-keyboard-context
npm install

Then check the manual article on how to replace the imports. You can now build your distro with the patch in the PR.

[valp124]

Thank you so much. Will work on it tomorrow. Isn't it around 3am where you are? :)

[andersevenrud]

Yeah... my stupid cat woke me up and now I'm having some trouble getting back to sleep :joy:

[valp124]

Melatonin!

[josephjeno]

Tart cherry juice helps too!

[josephjeno]

Just tested out the setKeyboardContext() method and can confirm it solved the issue!

[andersevenrud]

Great! I'll merge it into master then and make a new release :)

[andersevenrud]

Aaaaand, it's out :)

[valp124]

So just for completeness' sake: in this implementation, tabbing goes off the dialog if one is too eager. I suppose it was expected, based on our earlier exchange. This is also super insignificant! (One should never be too eager :wink: )

[andersevenrud]

I'll have to add some kind of "roll over" mechanism for tabbing, because once you reach the last element in the context it currently can't see what's next and might in some cases escape the context.

Using shift+tab will get it back into the right place again since that makes it go backwards.

This is true for windows as well. I'm opening a separate issue for this :)

https://github.com/os-js/osjs-client/issues/104

[valp124]

Indeed, shift+tab cycles in the reverse direction, I saw that!