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: resolve deps of useBackHandler #201

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

hosseinmd
Copy link

Summary

The method of using this Hook is not like React hooks and it needs to get deps in the last parameter like react hooks

useBackHandler(() => {
  if (isFocused) {
    setFocused(false);
    return true;
  }
  return false;
},[isFocused])

Test Plan

What's required for testing (prerequisites)?

What are the steps to reproduce (after prerequisites)?

Compatibility

OS Implemented
iOS
Android
Web

Checklist

  • I have tested this on a device and a simulator
  • I added the documentation in README.md
  • I updated the typed files (TS and Flow)

Copy link
Member

@pvinis pvinis left a comment

Choose a reason for hiding this comment

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

That's a good one, thanks!

@hosseinmd
Copy link
Author

This module has many PRs but why no one to organize them.

@LinusU LinusU added the major Increment the major version when merged label Nov 16, 2020
@hosseinmd
Copy link
Author

hosseinmd commented Dec 7, 2020

We can't manage backhandler when that is on the previous page,
Imagine you have Two Page by react-navigation. both pages have a back handler if the previous page a re-render occurred, Backhandler will add listener again so if the user clicks the back button previous page back handler will call
I think this PR could reduce these issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major Increment the major version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants