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

fix: add event emit for long press #3769

Merged
merged 3 commits into from
Mar 20, 2023
Merged

Conversation

josemak25
Copy link
Contributor

Summary

Fixes event emitter for #3758

Test plan

yarn bootstrap works

@josemak25 josemak25 requested a review from tjaniczek March 20, 2023 16:07
@josemak25 josemak25 changed the title Fix: add event emit for long press fix: add event emit for long press Mar 20, 2023
@callstack-bot
Copy link

callstack-bot commented Mar 20, 2023

Hey @josemak25, thank you for your pull request 🤗. The documentation from this branch can be viewed here.

@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here.

@lukewalczak lukewalczak merged commit baa0372 into main Mar 20, 2023
@lukewalczak lukewalczak deleted the fix/add-event-emit-for-long-press branch March 20, 2023 19:55
Copy link
Member

@satya164 satya164 left a comment

Choose a reason for hiding this comment

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

All this needed to do is add onTabLongPress={({ route }) => navigation.emit({ type: 'tabLongPress', target: route.key })}

This abstraction is unnecessary. There's also no default action that happens on onLongPress so canPreventDefault doesn't do anything.

@josemak25
Copy link
Contributor Author

All this needed to do is add onTabLongPress={({ route }) => navigation.emit({ type: 'tabLongPress', target: route.key })}

This abstraction is unnecessary. There's also no default action that happens on onLongPress so canPreventDefault doesn't do anything.

Noted making changes to this now

teneeto pushed a commit that referenced this pull request May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants