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

Remove useBootstrap #285

Closed
wants to merge 3 commits into from
Closed

Conversation

neftaly
Copy link
Collaborator

@neftaly neftaly commented Feb 1, 2024

This PR removes localStorage (document URL persistence) from useBootstrap. It is intended to enable useBootstrap to be used without a storage adapter, as per #266.

@neftaly
Copy link
Collaborator Author

neftaly commented Feb 1, 2024

It would be helpful to show an example of localStorage URL persistence in a tutorial or cookbook or similar

@neftaly neftaly changed the title Add useBootstrap enableLocalStorage option Remove localStorage from useBootstrap Feb 1, 2024
@pvh
Copy link
Member

pvh commented Mar 21, 2024

We talked about this a while back but I think we ought to just deprecate useBootstrap entirely. I agree that the localStorage is the pits but the longer I go, the more I lean towards doing away with it. What do you think?

@neftaly
Copy link
Collaborator Author

neftaly commented Mar 21, 2024

Sounds good, I would rather this existed in a tutorial or example.

@neftaly neftaly changed the title Remove localStorage from useBootstrap Remove useBootstrap Mar 21, 2024
@neftaly
Copy link
Collaborator Author

neftaly commented Mar 21, 2024

There is still an example using useBootstrap in packages/automerge-reac-hooks/README.md (and the identical comment on packages/automerge-reac-hooks/index.ts).

I don't have the bandwidth to fix the example though sorry! But it's not urgent and shouldn't affect anything.

@pvh
Copy link
Member

pvh commented Apr 4, 2024

I had a heap of rebase conflicts so I just reimplemented this but it's done now. Thanks @neftaly.

@pvh pvh closed this Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants