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

fixes addon on apps with engines #86

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

conradlz
Copy link

Fixed build failing in applications with ember-engines.

@eriktrom
Copy link
Member

update the yarn.lock file, thanks!

also, this is the same pr as html-next/ember-gestures#124

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

@RobbieTheWagner
Copy link
Member

@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
Copy link
Member

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
Copy link
Member

@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
Copy link
Member

eriktrom commented Jun 18, 2019

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
Copy link
Member

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
Copy link
Member

eriktrom commented Sep 4, 2019

@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

@eriktrom eriktrom closed this Sep 4, 2019
@RobbieTheWagner
Copy link
Member

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
Copy link
Member

eriktrom commented Sep 5, 2019

@rwwagner90 - agree

@conradlz
Copy link
Author

conradlz commented Sep 5, 2019

@eriktrom and @rwwagner90

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

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.

3 participants