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

chore(docs): identify that useLocalStorageValue is sync'd across tabs #1343

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

develohpanda
Copy link

What is the problem?

While reading through the migrate from react-use docs I incorrectly determined that the new useLocalStorageValue hook was not sync'd across tabs, because of this note

image

What changes does this PR make to fix the problem?

Explicitly identify in the useLocalStorageValue() docs that the value is kept in sync across tabs.

Checklist

  • Is there an existing issue for this PR?
    • link issue here

@xobotyi
Copy link
Contributor

xobotyi commented Aug 14, 2023

You'd better also fix the issue within migration guide https://github.com/react-hookz/web/blob/master/src/__docs__/migrating-from-react-use.story.mdx

Thanks for contribution

@xobotyi
Copy link
Contributor

xobotyi commented Aug 14, 2023

I believe synchronization been implemented after one of refactors of that hook🤔 Bu i forgot to update migration guide.

@develohpanda
Copy link
Author

develohpanda commented Aug 14, 2023

@xobotyi I thought that I had misread the migration guide.

Should this be referring to useLocalStorageValue() from this package, or useLocalStorage() from react-use?

image

(Presumably it is referring to useLocalStorageValue())

@xobotyi
Copy link
Contributor

xobotyi commented Aug 14, 2023

@develohpanda it shuld describe how it works within react-hookz version. After second read i've done it opposite way from the moment i've wrote the comment 🙃
IMO it would be better to rephraze whole note about sync

@develohpanda
Copy link
Author

Sounds good, thanks for confirming! I'll update and push to this branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants