-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: add geolocation hooks and mutations #590
Conversation
2da8dc4
to
0fc1baa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this PR. Do you think my suggestions make sens ? Would it help reduce the complexity of the code ?
const enabled = Boolean( | ||
(lat1 || lat1 === 0) && | ||
(lat2 || lat2 === 0) && | ||
(lng1 || lng1 === 0) && | ||
(lng2 || lng2 === 0), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading the type here it looks like this condition can never be false ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is false if one of the prop is null, or am I wrong?
On the contrary all values should be set (set at 0 is valid) to be true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ItemGelolocation['lat']
is a number
so they can not be null ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right but we never know aha
Quality Gate passedThe SonarCloud Quality Gate passed, but some issues were introduced. 1 New issue |
close #588