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.

Sign up for GitHub

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

Jump to bottom

Add an option to disable geolocation #51

Closed
tremby opened this issue Aug 17, 2022 · 2 comments · Fixed by #52
Closed

Add an option to disable geolocation #51

tremby opened this issue Aug 17, 2022 · 2 comments · Fixed by #52
Assignees
@bence-toth

Comments

@tremby
Copy link
Contributor

tremby commented Aug 17, 2022 •
edited
Loading

Best practice is to only try to geolocate the user on the user's request, or at least when it's clear to the user why the browser is asking for their location.

React doesn't let us conditionally use hooks, short of conditionally rendering the component itself which uses the hook. If I make a conditionally-rendered nested component and use the hook from there, I'd need to pass the location data up the tree back to the parent component, which is not the React way.

I'd like to see an enable (default true) or disable (default false) option, so I can still use the hook but not have it actually do anything until I flip that option in response to the user saying "geolocate me".
@bence-toth
Copy link
Owner

Hi @tremby, thanks for this, it makes perfect sense to me.

Would you mind opening a PR which adds this option?

tremby added a commit to tremby/react-hook-geolocation that referenced this issue Aug 18, 2022
@tremby
Add enable option
6d66a70 
An `enable` option is added as a new third argument, which defaults to
`true`. It can be set to `false` to disable geolocation. The idea is
that the option can be flipped to `true` once the user has indicated
that they want to be geolocated.

This means the hook can be made conditional without violating the rules
of hooks and without a conditional nested component which would need to
pass its data back up the tree.

Closes bence-toth#51
@tremby tremby mentioned this issue Aug 18, 2022
Merged
tremby added a commit to tremby/react-hook-geolocation that referenced this issue Aug 18, 2022
@tremby
Add enable option
767d0c7 
An `enable` option is added as a new third argument, which defaults to
`true`. It can be set to `false` to disable geolocation. The idea is
that the option can be flipped to `true` once the user has indicated
that they want to be geolocated.

This means the hook can be made conditional without violating the rules
of hooks and without a conditional nested component which would need to
pass its data back up the tree.

Closes bence-toth#51
@bence-toth
Copy link
Owner

Thank you @tremby 🙌

@bence-toth bence-toth self-assigned this Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bence-toth bence-toth

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add enable option tremby/react-hook-geolocation
2 participants
@tremby @bence-toth