-
Notifications
You must be signed in to change notification settings - Fork 21
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
feature: useStorage hook to persist data on frontend #83
base: main
Are you sure you want to change the base?
feature: useStorage hook to persist data on frontend #83
Conversation
hello sir @stephane-segning please waiting for review on this PR |
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.
This a good code, but the intended implementation. This doesn't make use of the react context features.
hello sir @stephane-segning so we will have a StorageContext.ts? inside the {root}/src/contexts folder |
…plement useStorage hook
hello sir @stephane-segning Added StorageContext global context for interacting with localStorage please review ths PR |
…lement useStorage hook
hello sir @stephane-segning please review this pull request |
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.
Create and managing state management inside of an app is always tricky. Working with react context is no exception. But we'll handle this. Take your time to review these points @AssahBismarkabah
hello sir @stephane-segning please waiting for your review on this PR |
hello sir @stephane-segning please review this changes |
hello sir @stephane-segning waiting for your review |
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.
Nice work @AssahBismarkabah @Christiantyemele ! Just these few points now
}, | ||
removeItem: async (key) => clearItem(key), | ||
clear: async () => { | ||
localStorage.clear(); |
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.
Remove this line too
if (storedValue !== undefined) { | ||
setItem(key, storedValue); | ||
} |
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.
Here you can remove the if
condition there
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.
ok sir @stephane-segning
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.
You didn't removed this if
if (storedValue !== undefined) { | ||
setItem(key, storedValue); | ||
} |
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.
You didn't removed this if
hello sir @stephane-segning I have added the docs for the indexDB storage and requesting for review before further do |
@AssahBismarkabah and @Motouom request and @stephane-segning please requesting for review on this PR |
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.
Okay!
const [storedValue, setStoredValue] = useState<T | undefined>(() => { | ||
return item[key] ?? initialValue; | ||
}); |
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.
Remove this and use item[key]
directly instead of 'storedValue'
@AssahBismarkabah @Christiantyemele please consider these changes that @stephane-segning asked |
added useStorage to persist data on the frontend