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

Keyboard input in perseus does not work with touchscreen devices #11891

Closed
marcellamaki opened this issue Feb 16, 2024 · 14 comments · Fixed by #11998
Closed

Keyboard input in perseus does not work with touchscreen devices #11891

marcellamaki opened this issue Feb 16, 2024 · 14 comments · Fixed by #11998
Assignees
Labels
P1 - important Priority: High impact on UX

Comments

@marcellamaki
Copy link
Member

marcellamaki commented Feb 16, 2024

Spotted by @jamalex during the 0.16 sync session on Feb 14 2024

Observed behavior

When using a touch-screen device (such as a laptop that features a touch screen), our implementation of perseus currently does not accept the option for both touch and keyboard input. Perseus requires us to set whether it's a mobile display, and we use isTouch to do this. When set to mobile display, clicking on text input boxes in an exercise does not set focus or allow text to be entered.

Expected behavior

We should update the condition to isTouch && !isKeyboardAndMouse in order to set whether it is a mobile display.

Context

0.16.x

@marcellamaki marcellamaki added the P1 - important Priority: High impact on UX label Feb 16, 2024
@poju3185
Copy link
Contributor

Hi @marcellamaki and @rtibbles, could I work on this issue? I'm thinking implement isKeyboardAndMouse in browswerInfo.js and change useTouch in PersusRenderIndex.vue to

usesTouch() {
  return isTouchDevice && !isKeyboardAndMouse;
},

@rtibbles
Copy link
Member

Hi @poju3185 - you are welcome to - I would be interested in how you plan to set the value of the isKeyboardAndMouse boolean?

@poju3185
Copy link
Contributor

poju3185 commented Mar 19, 2024

After exploring several approaches, I am using a solution that involves updating the keyboardUsed value in localStorage to true upon detecting a keydown event, and then refreshing the page. Like so

      handleKeyDown(event) {
        console.log("keydown event detected");
        localStorage.setItem('keyboardUsed', 'true');
        this.keyboardUsedState = true;
      },
      usesTouch() {
        return isTouchDevice && !this.keyboardUsedState;
      },
 watch: {
      usesTouch(newVal, oldVal) {
        if (newVal !== oldVal) {
          window.location.reload();
        }
      },
  }

I believe the idael way would be to directly detect the presence of a physical keyboard. Unfortunately, based on my research, it seems JavaScript doesn't have a straightforward way to accomplish this.

My second attempt involved invoking renderItem when detecting changes in usesTouch. However, this approach led to bugs, please refer to the video below:

Screen.Recording.2024-03-18.at.11.28.16.PM.mov

Ultimately, I decided that refreshing the page was the most reliable solution to ensure the UI correctly reflects the current input method.

Screen.Recording.2024-03-18.at.11.30.33.PM.mov

@rtibbles
Copy link
Member

I think there may still be a problem with this approach - which is that an on screen keyboard (like the Android on screen keyboard) will still emit keydown events, and so would be registered as a keyboard by this logic. This would fix the issue at hand, but would render the touch detection pointless, and would mean that we never show the special math input keyboard.

@rtibbles
Copy link
Member

One possibility might be to instead try to detect a mouse: https://stackoverflow.com/a/70827986 - we can use the pointerType property of the PointerEvent to distinguish between mouse clicks and touch events.

@poju3185
Copy link
Contributor

Got it, thank you for the clarification. So, to summarize, the system will switch back to the standard mode when the user interacts using mouse clicks, and it will switch again to mobile mode when the user is interacting via touch, right?

@rtibbles
Copy link
Member

That feels more complex, I think for now, we can just use whether there has been a mouse click event to set the keyboardAndMouse boolean - and then just use it as suggested above.

@poju3185
Copy link
Contributor

Okay, I'm on it!

@poju3185
Copy link
Contributor

poju3185 commented Mar 19, 2024

Here's the updated version, where it can distinguish mouse click from touch event, only switch back to standard mode when detected a mouse click event.

Screen.Recording.2024-03-19.at.10.08.53.AM.mov

@rtibbles
Copy link
Member

I think I would rather avoid doing the page refresh - let's just set up the mouse click detection logic globally in the browserInfo module, and then just assume that if someone has got to this page, they will have had to have clicked the mouse to get there.

This should cover 99% of the cases - without having to deal with doing a refresh on mouse click.

@poju3185
Copy link
Contributor

I agree that refreshing the page would be inconvenient for users.

@poju3185
Copy link
Contributor

In this version, as long as users click with their mouse anywhere in the app, even if they use touch to enter the exercise, Perseus will still render in standard mode.

Screen.Recording.2024-03-19.at.10.47.38.AM.mov

@rtibbles
Copy link
Member

That seems like the best we can do, thank you! Please open a PR, and I'll review the code.

rtibbles added a commit that referenced this issue Mar 22, 2024
Fix #11891 Enhanced Input Mode Detection for Hybrid Devices
@rtibbles
Copy link
Member

Fixed in #11998

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 - important Priority: High impact on UX
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants