Fix #11891 Enhanced Input Mode Detection for Hybrid Devices #11998

poju3185 · 2024-03-19T16:15:02Z

Summary

Fix #11891
This Pull Request introduces an enhancement to the input mode detection logic for devices with both touch and mouse inputs. The core of this update is a global mouse click listener across the app, ensuring that once a mouse click is detected, Perseus will consistently render in standard mode. This approach simplifies the input mode logic and provides a more predictable user experience on hybrid devices.

References

https://stackoverflow.com/a/70827986 provided by @rtibbles

Reviewer guidance

On a hybrid device or using device emulation, open the app and navigate to a Perseus exercise.
Initially use touch input to interact with the app and observe Perseus rendering in mobile mode.
Click anywhere in the app using the mouse or trackpad.
Navigate to the same or a different Perseus exercise and verify that it now renders in standard mode, reflecting the detected mouse input.
Attempt to switch back to touch input and observe that Perseus continues to render in standard mode, demonstrating the mouse input precedence.

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

Testing checklist

Contributor has fully tested the PR manually
If there are any front-end changes, before/after screenshots are included
Critical user journeys are covered by Gherkin stories
Critical and brittle code paths are covered by unit tests

PR process

PR has the correct target branch and milestone
PR has 'needs review' or 'work-in-progress' label
If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
If this is an important user-facing change, PR or related issue has a 'changelog' label
If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

Automated test coverage is satisfactory
PR is fully functional
PR has been tested for accessibility regressions
External dependency files were updated if necessary (yarn and pip)
Documentation is updated
Contributor is in AUTHORS.md

rtibbles · 2024-03-20T04:19:50Z

Sorry @poju3185, I forgot to mention that we are targeting this fix to 0.16. I have retargeted the PR (please ignore the diff from that). Please follow the instructions here: https://kolibri-dev.readthedocs.io/en/develop/howtos/rebasing_a_pull_request.html
To rebase your pull request onto the release-v0.16.x branch.

poju3185 · 2024-03-20T04:29:10Z

No worries. I've already rebased the pull request onto the release-v0.16.x branch as instructed. Thank you for the guidance!

github-actions · 2024-03-20T05:14:23Z

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.16.1.dev0_git.20240322152344.pex
Windows Installer (EXE)	kolibri-0.16.1.dev0+git.20240322152344-unsigned.exe
Debian Package	kolibri_0.16.1.dev0+git.20240322152344-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.16.1.dev0+git.20240322152344-0.4.0.dmg
Android Package (APK)	kolibri-0.16.1.dev0+git.20240322152344-0.1.0-debug.apk
TAR file	kolibri-0.16.1.dev0+git.20240322152344.tar.gz
WHL file	kolibri-0.16.1.dev0+git.20240322152344-py2.py3-none-any.whl

rtibbles

This definitely works, but I think I'd rather make the API a bit more explicit and hide the implementation details from the API consumer.

rtibbles · 2024-03-21T14:56:10Z

kolibri/core/assets/src/kolibri_app.js

@@ -58,6 +58,12 @@ export default class KolibriApp extends KolibriModule {
    return {};
  }

+  handlePointerDown(event) {


I think I would rather move all of this logic into the browserInfo.js module - and create a simple exportable boolean in there mousePointerUsed or something like that.

This can be initialized from the localStorage initially, and then we can update both the localStorage and the value when the condition below happens.

rtibbles · 2024-03-21T14:57:05Z

kolibri/core/assets/src/kolibri_app.js

@@ -110,6 +116,10 @@ export default class KolibriApp extends KolibriModule {
        ...this.stateSetters.map(setter => setter(this.store)),
      ]).then(() => {
        this.startRootVue();
+        window.addEventListener('pointerdown', this.handlePointerDown.bind(this));


This can be done in the module level of browserInfo.js - it will only get executed once, so it feels safe to me.

In terms of unbinding, once we have detected this once, we don't need to be checking any more, so at that point we could remove the event listener.

rtibbles · 2024-03-21T14:57:42Z

kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue

@@ -144,7 +144,7 @@
        return this.windowBreakpoint < 3;
      },
      usesTouch() {
-        return isTouchDevice;
+        return isTouchDevice && localStorage.getItem('mouseUsed') !== 'true';


As I commented above, let's add a browserInfo exportable variable that will define this value - so that the implementation details are hidden from the code consumer here.

Signed-off-by: PoJuDeSu <spes9850401@gmail.com>

poju3185 · 2024-03-21T16:29:46Z

I've made the updates, please notice that isMouseUsed is implemented as a function rather than a constant because its value may change over time.

rtibbles

Just one last thing to have a referent rather than a function for isMouseUsed.

rtibbles · 2024-03-21T19:54:42Z

kolibri/core/assets/src/utils/browserInfo.js

+
+window.addEventListener('pointerdown', handlePointerDown);
+
+export function isMouseUsed() {


One of the nice things about Javascript exports is that they are referential - this has the nice effect of being able to export dynamic variables from a module.

This means that we don't have to have isMouseUsed as a function, instead it can be initialized as:

export let isMouseUsed = localStorage.getItem('mouseUsed') === 'true';

rtibbles · 2024-03-21T19:55:03Z

kolibri/core/assets/src/utils/browserInfo.js

+
+function handlePointerDown(event) {
+  if (event.pointerType === 'mouse') {
+    localStorage.setItem('mouseUsed', 'true');


Then in here we can do this and:

isMouseUsed = true;

Signed-off-by: PoJuDeSu <spes9850401@gmail.com>

poju3185 · 2024-03-21T22:30:19Z

I've made the suggested change to initialize isMouseUsed using the export with the dynamic variable approach. Now the code looks better.

rtibbles

This looks good! I'll ask our QA team to give it a quick test if possible, just to make sure it's all working as intended.

All addressed!

pcenov · 2024-03-22T17:00:04Z

Hi @rtibbles - I've just tested this on a Chromebook laptop and I can confirm that I can input an answer both when using the touch screen or the trackpad. However when using only the touch screen I still have to type the answer using the physical keyboard of the device while I was expecting to see the onscreen numeric keyboard. The same is valid when I flip the physical keyboard of the Chromebook and start using it as a tablet in which case instead of seeing the onscreen numeric keyboard I see the normal digital keyboard.

rtibbles · 2024-03-22T17:13:26Z

Thanks @pcenov - I think what you describe is sufficient to fix the issue at hand (being able to input at all in this scenario) but we can file a follow up issue for the behaviour that is more responsive to the current state of the device. This more responsive functionality will be significantly harder to implement.

rtibbles · 2024-03-22T22:37:02Z

Follow up filed here: #12016

rtibbles

Great work, thank you!

github-actions bot added DEV: renderers HTML5 apps, videos, exercises, etc. DEV: frontend SIZE: small labels Mar 19, 2024

rtibbles changed the base branch from develop to release-v0.16.x March 20, 2024 04:17

detect mouse click accross the whole app

69a7adc

poju3185 force-pushed the detect-keyboard-in-perseus branch from b6d9be7 to 69a7adc Compare March 20, 2024 04:27

MisRob requested review from rtibbles and nucleogenesis March 20, 2024 07:33

rtibbles previously requested changes Mar 21, 2024

View reviewed changes

rtibbles self-assigned this Mar 21, 2024

poju3185 added 2 commits March 21, 2024 11:23

Hide implementation of isMouseUsed

95d24a6

Signed-off-by: PoJuDeSu <spes9850401@gmail.com>

remove unnecessary lines

3f7c981

Signed-off-by: PoJuDeSu <spes9850401@gmail.com>

rtibbles reviewed Mar 21, 2024

View reviewed changes

update isMouseUsed as a variable

158dc40

Signed-off-by: PoJuDeSu <spes9850401@gmail.com>

rtibbles reviewed Mar 22, 2024

View reviewed changes

add new line at end of file

b8aae9f

rtibbles mentioned this pull request Mar 22, 2024

Perseus renderer does not responsively update to the current input modality #12016

Open

rtibbles approved these changes Mar 22, 2024

View reviewed changes

rtibbles merged commit b16c589 into learningequality:release-v0.16.x Mar 22, 2024
34 checks passed

rtibbles mentioned this pull request Mar 22, 2024

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

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #11891 Enhanced Input Mode Detection for Hybrid Devices #11998

Fix #11891 Enhanced Input Mode Detection for Hybrid Devices #11998

poju3185 commented Mar 19, 2024 •

edited

rtibbles commented Mar 20, 2024

poju3185 commented Mar 20, 2024

github-actions bot commented Mar 20, 2024 •

edited

rtibbles left a comment

rtibbles Mar 21, 2024

rtibbles Mar 21, 2024

rtibbles Mar 21, 2024

poju3185 commented Mar 21, 2024

rtibbles left a comment

rtibbles Mar 21, 2024

rtibbles Mar 21, 2024

poju3185 commented Mar 21, 2024 •

edited

rtibbles left a comment

pcenov commented Mar 22, 2024

rtibbles commented Mar 22, 2024

rtibbles commented Mar 22, 2024

rtibbles left a comment


		window.addEventListener('pointerdown', handlePointerDown);

		export function isMouseUsed() {

Fix #11891 Enhanced Input Mode Detection for Hybrid Devices #11998

Fix #11891 Enhanced Input Mode Detection for Hybrid Devices #11998

Conversation

poju3185 commented Mar 19, 2024 • edited

Summary

References

Reviewer guidance

Testing checklist

PR process

Reviewer checklist

rtibbles commented Mar 20, 2024

poju3185 commented Mar 20, 2024

github-actions bot commented Mar 20, 2024 • edited

Build Artifacts

rtibbles left a comment

Choose a reason for hiding this comment

rtibbles Mar 21, 2024

Choose a reason for hiding this comment

rtibbles Mar 21, 2024

Choose a reason for hiding this comment

rtibbles Mar 21, 2024

Choose a reason for hiding this comment

poju3185 commented Mar 21, 2024

rtibbles left a comment

Choose a reason for hiding this comment

rtibbles Mar 21, 2024

Choose a reason for hiding this comment

rtibbles Mar 21, 2024

Choose a reason for hiding this comment

poju3185 commented Mar 21, 2024 • edited

rtibbles left a comment

Choose a reason for hiding this comment

pcenov commented Mar 22, 2024

rtibbles commented Mar 22, 2024

rtibbles commented Mar 22, 2024

rtibbles left a comment

Choose a reason for hiding this comment

poju3185 commented Mar 19, 2024 •

edited

github-actions bot commented Mar 20, 2024 •

edited

poju3185 commented Mar 21, 2024 •

edited