search

html-next / ember-hammertime

TouchAction (aka "fastclick") Support for Ember Applications
MIT License
56 stars 23 forks source link

fixes addon on apps with engines #86

Open conradlz opened 5 years ago

conradlz commented 5 years ago

Fixed build failing in applications with ember-engines.

eriktrom commented 5 years ago

update the yarn.lock file, thanks!

also, this is the same pr as https://github.com/html-next/ember-gestures/pull/124

I can merge but not release, so i'll defer to @rwwagner90

RobbieTheWagner commented 5 years ago

@eriktrom I feel like we should deprecate this package. I don't think it is necessary for modern web development anymore to add fast click hacks. I suppose for those still supporting IE maybe.

eriktrom commented 5 years ago

I feel like we should deprecate this package. I don't think it is necessary for modern web development anymore to add fast click hacks

I'm not opposed tbh - it was only for fast click on safari touch to begin with right? (thus don't think IE is an issue?)

RobbieTheWagner commented 5 years ago

@eriktrom I'm really not sure. I know all modern browsers and mobile browsers are supposedly the same, so I was just throwing out the possibility maybe someone supporting old browsers would need this.

If it only fixed mobile Safari specifically, I say we deprecate. Thoughts @runspired?

eriktrom commented 5 years ago

It was abstracted from ember-gestures a while ago - it was only for the 300ms delay on ios (are you hovering... 300ms... no, okay you actually want to 'click' that input, is basically what it did).

However, I'm not sure if it's still relevant for the older web views on ios. Finally, ember-gestures has a dep on it, which is in our hands, I can handle that, if we decide to deprecated this.

and agree, @runspired -- in which version of ios did this become irrelevant - there is a webkit blogpost from a couple years back that claimed 300ms delay would be removed, but webkit tickets (of which i don't have links to on hand atm) did indicate at that time, that it wasn't fully removed, I'd need to find such tickets and their status to confirm, but ideally, you just know everything™️ so we'll decide based on your feedback. If I can help track things down, let me know. Cheers.

RobbieTheWagner commented 5 years ago

I know I haven't noticed any issues on mobile anymore, so whether it was officially fixed or not, I think it is no longer an issue.

eriktrom commented 5 years ago

@PrinceCornNM - make a perf profile .json file in safari when it's hooked up to the ios device in question. Click around, try to get the timeline to show the 300ms delay and I'll re-open.

Thanks

RobbieTheWagner commented 5 years ago

I think this was just to fix the build, which we could maybe merge, but we need to decide if we should deprecate this whole package.

eriktrom commented 5 years ago

@rwwagner90 - agree

conradlz commented 5 years ago

@eriktrom and @rwwagner90

It fixed build, and it enabled me to use this in our library for our app.