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

Support clicks within draggable elements on iOS Safari #327

Merged
merged 4 commits into from Sep 23, 2019

Conversation

@amannn
Copy link
Contributor

commented Sep 18, 2019

Fixes #322 .

I found out that when you don't preventDefault the touchstart event, Safari will try to scroll the window. I found your blockViewportScroll utility, but it didn't seem to work. Apparently you need to register the event handler on document instead of window. Now it works properly in my simulator. Not sure if there's a case for also handling events from window?

For the check if the device supports touch events, I used the existing function in use-pointer-event. Is that ok?

amannn added 2 commits Sep 18, 2019
Copy link
Collaborator

left a comment

Hey @amannn thanks for the PR! This looks great, just a couple of comments

// Make sure input elements loose focus when we prevent the default.
if (document.activeElement instanceof HTMLElement) {
document.activeElement.blur()
if (!supportsTouchEvents()) {

This comment has been minimized.

Copy link
@InventingWithMonster

InventingWithMonster Sep 20, 2019

Collaborator

Could you add a note as to why we're adding this text (describe the bug we're avoiding)

@@ -5,7 +5,8 @@ import { wrapHandler, EventListenerWithPointInfo } from "./event-info"
// We check for event support via functions in case they've been mocked by a testing suite.
const isBrowser = typeof window !== "undefined"
const supportsPointerEvents = () => isBrowser && window.onpointerdown === null
const supportsTouchEvents = () => isBrowser && window.ontouchstart === null
export const supportsTouchEvents = () =>

This comment has been minimized.

Copy link
@InventingWithMonster

InventingWithMonster Sep 20, 2019

Collaborator

Could you move these supports functions to a events/utils file?

…ove event utils to separate module.
@amannn

This comment has been minimized.

Copy link
Contributor Author

commented Sep 20, 2019

Thanks for the feedback! I've added a commit which addresses both points.

@InventingWithMonster

This comment has been minimized.

Copy link
Collaborator

commented Sep 23, 2019

Excellent, thanks!

@mergetron mergetron bot merged commit 75f6fb5 into framer:master Sep 23, 2019
3 checks passed
3 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
license/snyk - package.json (Framer) No manifest changes detected
security/snyk - package.json (Framer) No manifest changes detected
@amannn

This comment has been minimized.

Copy link
Contributor Author

commented Sep 24, 2019

Thanks for the review and merge! Looking forward to the release 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.