FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(iOS-gestures) touch delegate does not call base class touch methods by farfromrefug · Pull Request #6113 · NativeScript/NativeScript · GitHub

/ NativeScript Public

fix(iOS-gestures) touch delegate does not call base class touch methods - #6113

Merged
vakrilov merged 6 commits into
NativeScript:masterfrom
Akylas:ios_touch_fix
Aug 2, 2018
ConversationCommits (6)ChecksFiles changed
Merged

fix(iOS-gestures) touch delegate does not call base class touch methods#6113
vakrilov merged 6 commits into
NativeScript:masterfrom
Akylas:ios_touch_fix

Conversation

farfromrefug commented Jul 25, 2018 •
edited by vakrilov
Loading

Copy link
Copy Markdown
Collaborator

PR Checklist

What is the current behavior?

Attach touch gesture to a button (best example to see the issue). You won't get button animations anymore.

What is the new behavior?

The touch gesture delegate now calls base methods on the native observed view.

Fixes #4449

You can test it here: https://play.nativescript.org/?template=play-ng&id=46hl3N&v=2

ghost added the ♥ community PR label Jul 25, 2018
ghost assigned vakrilov Jul 30, 2018
ghost added in progress and removed ♥ community PR labels Jul 30, 2018
vakrilov suggested changes Jul 30, 2018
View reviewed changes
Comment thread tns-core-modules/ui/gestures/gestures.ios.ts Outdated

touchesBeganWithEvent(touches: NSSet<any>, event: any): void {
this.executeCallback(TouchAction.down, touches, event);
(this.observer.target.nativeView as UIView).touchesBeganWithEvent(touches, event);

vakrilov Jul 30, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can you use this.view (type is UIView) -> it should be the same, only shorter and without the casting.

farfromrefug Jul 30, 2018

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

sure didn't know I could access it that way

farfromrefug Jul 30, 2018

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@vakrilov done!

vakrilov commented Jul 30, 2018

Copy link
Copy Markdown
Contributor

test

vakrilov commented Jul 30, 2018

Copy link
Copy Markdown
Contributor

@farfromrefug Looks good - thanks for the PR. Can just check my review.

vakrilov commented Jul 31, 2018

Copy link
Copy Markdown
Contributor

test

farfromrefug commented Aug 1, 2018

Copy link
Copy Markdown
Collaborator Author

@vakrilov do I have to do something? I can see that some tests are failing when they were passing before.

vakrilov commented Aug 1, 2018

Copy link
Copy Markdown
Contributor

test

vakrilov commented Aug 1, 2018

Copy link
Copy Markdown
Contributor

Hey @farfromrefug - there is an unstable test in the CI that is causing some of the PR builds to fail. We are currently hunting it down.
There is nothing you can do - I don't think this is a problem with this PR.

SvetoslavTsenov commented Aug 1, 2018

Copy link
Copy Markdown
Contributor

test

farfromrefug commented Aug 1, 2018

Copy link
Copy Markdown
Collaborator Author

@vakrilov thanks for explaining. I think I also understand your "test" comments now. It s there to trigger a test build!

vakrilov commented Aug 1, 2018

Copy link
Copy Markdown
Contributor

You are correct - test just triggers the internal Jenkins builds.
Although, if you don't know about it, it's easy to take it the wrong way:

vakrilov merged commit 7ebac7c into NativeScript:master Aug 2, 2018
ghost removed the in progress label Aug 2, 2018
farfromrefug deleted the ios_touch_fix branch August 2, 2018 09:49
shiv19 mentioned this pull request Aug 14, 2018
Closed

shiv19 commented Aug 14, 2018

Copy link
Copy Markdown
Member

@farfromrefug Thanks! 😄
I was staring at that part of gestures.ios.ts not being able to understand why it isn't working.
And then I noticed that that file was changed 13 days ago, so the changes aren't in @latest version of tns-core-modules.

farfromrefug commented Aug 15, 2018

Copy link
Copy Markdown
Collaborator Author

@shiv19 use @next

shiv19 commented Aug 15, 2018

Copy link
Copy Markdown
Member

Yes. But I was trying to use this for a NativeScript snack. Which is on NativeScript Playground.
Because of that, I'll have to wait till this change reaches the @latest version, and is upgraded on the playground.

vakrilov pushed a commit that referenced this pull request Sep 11, 2018
fix(iOS-gestures) touch delegate does not call base class touch metho…
284cd54 
…ds (#6113)

* fix ios Touch gestures super methods not being called on nativeView

* use “this.view”
edusperoni mentioned this pull request Oct 5, 2018
Closed

lock Bot commented Nov 2, 2019

Copy link
Copy Markdown

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

lock Bot locked and limited conversation to collaborators Nov 2, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
vakrilov vakrilov approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

vakrilov

Labels

cla: yes ♥ community PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

@HostListener with touch event in directive is preventing the tap event in component (Only in iOS)

6 participants

Back | FazBrowse Home | New Git URL