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

Polyfilling pointer-events #4095

Closed
tsi opened this issue Aug 14, 2023 · 17 comments
Closed

Polyfilling pointer-events #4095

tsi opened this issue Aug 14, 2023 · 17 comments

Comments

@tsi
Copy link

tsi commented Aug 14, 2023

First, would like to say thanks for all the work you people put into this.

Now, we're developing a feature relying on pointer-events so that we can handle both mouse and touch events easily.
On old Safari, a polyfill is required for those events to function.
The polyfill seems to work on many demos but I couldn't get it to work in my project.
I tested multiple directions and eventually got to the conclusion that it's probably Preact that is in my way.
Here are two sandboxes:
This one is using React and works on Safari 12 with the polyfill
This one has the exact same component but using Preact, not responsive on Safari 12

Can anyone help explain the issue? Is there anything we can do about it?
Thanks!

@rschristian
Copy link
Member

rschristian commented Aug 14, 2023

It looks like both of your links are to the same sandbox, and it doesn't look like you're using any polyfill either?

@tsi
Copy link
Author

tsi commented Aug 14, 2023

Sorry, the one using Preact is this one. edited the original msg. thanks!

@rschristian
Copy link
Member

rschristian commented Aug 14, 2023

Thanks for the fix!

Looks like onGotPointerCapture and onLostPointerCapture get their names mangled due to the useCapture check / name coercion:

useCapture = name !== (name = name.replace(/Capture$/, ''));

This stops the listener from being set correctly, as the event names end up as GotPointer and LostPointer.

They might need to be special cased here.

Edit: TS won't like it (though you can patch the types or // @ts-ignore), but you can use ongotpointercapture and onlostpointercapture (lower-cased) in the mean time.

@tsi
Copy link
Author

tsi commented Aug 15, 2023

Wow you already have a PR. that's truly amazing!
Just to make sure we are on the same page - my issue was with the other pointer-events handlers, i.e. onPointerDown, onPointerMove etc, not onGotPointerCapture/ onLostPointerCapture
The polyfill for Safari don't work for some reason, while it does work in other places.
Will this fix that?

@rschristian
Copy link
Member

rschristian commented Aug 15, 2023

This would do nothing to address those handlers, no.

Can you provide a reproduction? As I previously mentioned, we can't really help you debug your polyfill if you don't include it in the reproduction.

@tsi
Copy link
Author

tsi commented Aug 15, 2023

Oh I really hoped it would...
The examples above already include the polyfill and reproduce the issue, you'll need to open them in Safari 12 to see it (I used BrowserStack)

@rschristian

This comment was marked as outdated.

@tsi

This comment was marked as outdated.

@rschristian
Copy link
Member

rschristian commented Aug 15, 2023

Ah, well I'm clearly an idiot. Sorry about that! Will take a look a bit later.

Edit: Never mind, I have no way of easily reproducing Safari 12 or alternatives. It's probably an issue with the polyfill, as after #4096, all of those events seem to be working correctly in modern browsers.

@tsi
Copy link
Author

tsi commented Aug 16, 2023

The point was that the polyfill works fine with React's onPointerDown, onPointerMove etc
And also with addEventListener() pointerdown, pointermove etc
But not with Preact onPointer... props.
Is there anything we can try to make this work?

@rschristian
Copy link
Member

rschristian commented Aug 16, 2023

Preact certainly is using addEventListener.

Keep in mind there is a pretty big gap in React and Preact when it comes to events, in that Preact uses native DOM events while React does not. I have no idea if that's relevant to this issue but it's good to keep in mind that there are some intentional differences in this area.

Can you try using lowercase names for the events in your JSX? onpointerdown and whatnot

@tsi
Copy link
Author

tsi commented Aug 16, 2023

Can you try using lowercase names for the events in your JSX? onpointerdown and whatnot

I love you

@rschristian
Copy link
Member

I take it that works then?

Unfortunately that's what you'll need to use, Preact will only switch on* props to be lowercased when they're events that can actually be found in the DOM.

@tsi
Copy link
Author

tsi commented Aug 16, 2023

Yes, excuse my excitement :)
Just did a quick POC using BrowserStack. and it seems to work.

@rschristian
Copy link
Member

Glad to hear it!

I don't think there's any action we can take on the Preact side, this is the troublesome line:

preact/src/diff/props.js

Lines 92 to 93 in 21a6f30

// Infer correct casing for DOM built-in events:
if (name.toLowerCase() in dom) name = name.toLowerCase().slice(2);

I don't believe it's safe to call .toLowerCase() blindly, hence the DOM check.

You certainly could patch this behavior, adding || /^onPointer/.test(name) or similar to the conditional, else, you can use the following to fix the TS types to allow onpointer*:

declare module 'preact' {
  namespace JSX {
    interface DOMAttributes<Target extends EventTarget> {
      onpointerdown?: PointerEventHandler<Target> | undefined;
    }
  }
}

Just copying the source types, but changing event capitalization.

@tsi
Copy link
Author

tsi commented Aug 17, 2023

I really don't mind having it in lowercase, as long as it works on modern browsers, and as long as no other dev sees it and "fixes" it to camelCase.
When we'll be able to drop support for Safari 12 and below, we'll remove the polyfill and fix the pointer-event props to be camelCase.

Thank you so much for the responsiveness here, truly impressive.

@rschristian
Copy link
Member

Going to close this out in that case, let me know if you run into any further issues and we can reopen.

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

No branches or pull requests

2 participants