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

Add alternate touch event models #52

Merged
merged 10 commits into from
Jun 13, 2024

Conversation

abrin
Copy link
Contributor

@abrin abrin commented Jun 9, 2024

@stephenwf I've found that adding a touch-model interaction on-top of the canvas-panel isn’t working. Instead, it would work better if it was built into the Atlas interaction model. I'm more than open to thoughts and ideas here too.

This helps address some of this more cleanly:

  1. it addresses the fact that the browser-event-manager registers duplicate event listeners without removing them

  2. it removes the touch-action: none; and pointer-events: none; which was preventing the window from listening to scroll based touches.

  3. it replaces the above with making sure that e.preventDefault() is called when we don’t want to scroll because this does the same thing see this:

    Applications using Touch events disable the browser handling of gestures by calling preventDefault(), but should also use touch-action to ensure the browser knows the intent of the application before any event listeners have been invoked
4. With this set, we can now use `e.preventDefault()` in various places to determine when we want the event to be passed to the window.
  1. I also added some custom user-select settings to make sure that a long touch doesn’t try to select in Safari

  2. I also found that binding touch-move to the window was causing issues, so I bound it to the parent element of the canvas.

New interaction modes:

  1. ignoreSingleFingerTouch when this is enabled a single finger touch does nothing, instead of panning, it allows the window to scroll.

  2. enablePanOnWait in this interaction model it assumes that a user will wait a brief moment between when the press and when the move when the intent is to “pan.”

**Question: can I modify the popMotionController arguments in CanvasPanel — is this a matter of defining and setting them in the useAtlas setup in CanvasPanel?

abrin added 2 commits June 5, 2024 08:27
we’ve found that adding a touch-model interaction on-top of the canvas-panel isn’t working. Instead, it would work better if it was built into the Atlas interaction model.

This helps address some of this more cleanly:
1. it addresses the fact that the browser-event-manager registers duplicate event listeners without removing them

2. it removes the `touch-action: none;` and `pointer-events: none;` which was preventing the window from listening to scroll based touches.

3. it replaces the above with making sure that `e.preventDefault()` is called when we don’t want to scroll because this does the same thing [see this](https://developer.mozilla.org/en-US/docs/Web/CSS/touch-action) `Applications using Touch events disable the browser handling of gestures by calling preventDefault(), but should also use touch-action to ensure the browser knows the intent of the application before any event listeners have been invoked`

4. With this set, we can now use `e.preventDefault()` in various places to determine when we want the event to be passed to the window.

5. I also added some custom `user-select` settings to make sure that a long touch doesn’t try to select in Safari

6. I also found that binding touch-move to the window was causing issues, so I bound it to the parent element of the canvas.

New interaction modes:

1. **ignoreSingleFingerTouch** when this is enabled a single finger touch does nothing, instead of panning, it allows the window to scroll.

2. **enablePanOnWait** in this interaction model it assumes that a user will wait a brief moment between when the press and when the move when the intent is to “pan.”

**Question: can I modify the popMotionController arguments in CanvasPanel — is this a matter of defining and setting them in the `useAtlas` setup in CanvasPanel?
Copy link

codesandbox-ci bot commented Jun 10, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@stephenwf
Copy link
Collaborator

This looks great @abrin! Solves a lot of long-standing issues things in one go. I'll read in more depth soon.

@abrin abrin marked this pull request as ready for review June 13, 2024 15:59
@abrin
Copy link
Contributor Author

abrin commented Jun 13, 2024

@stephenwf I think this one is ready. We're trying to test and fine-tune what the timing should be, but the way that it's configured, this can be controlled though CanvasPanel now --> digirati-co-uk/iiif-canvas-panel#264 . I believe I've flipped the defaults all back to the point that they match the current behaviors.

@stephenwf
Copy link
Collaborator

Thanks @abrin, having a look now!

@stephenwf
Copy link
Collaborator

stephenwf commented Jun 13, 2024

@abrin looks good! I think I'll be using these options by default, but I agree that keeping it the same makes sense 👍 Do you want me to go ahead and merge?

I'll need to merge it into the 2.0.x branch too, but should be smooth.

The current main canvas panel branch is on 2.0.x of this repository but the PR I've opened gets everything back up to the latest versions of everything.

digirati-co-uk/iiif-canvas-panel#237

@abrin
Copy link
Contributor Author

abrin commented Jun 13, 2024

@stephenwf I think this work is ready. As with anything, there may be bugs and edge-cases... but in all the testing we've not seen any in particular. I will let you know where we end up with the interaction model and the timing. We're going to run some user-testing internally that helps us refine what the delay should be.

@stephenwf
Copy link
Collaborator

That would be great, thanks. I'll get this merged up.

@stephenwf stephenwf merged commit f526bf9 into atlas-viewer:main Jun 13, 2024
1 check passed
@abrin abrin deleted the add_alternate_touch_event_models branch June 13, 2024 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants