ts5-channel-image-generator
ts5-channel-image-generator copied to clipboard
localStorage Features
This PR Features:
- Updated dependencies
- Added saving image to localStorage
- Added saving options (not room structure yet) to localStorage
PS: I'm sorry if my code is not 100% efficient, I'm more a Vue.js than a React guy ;)
Hi, Thank you for making the PR :+1: I will try to review it later this week.
Hi, I have looked through it and it looks good. :+1:
The only issue is that there are some visual problems with inputs and other elements. It looks like blueprint JS has changed from using blue-ish colors to gray-ish colors and now it does not look that great with TeamSpeak blue background.
Thank you for looking through my PR!
I added a new commit to improve the looks of those elements. I realized that I forgot to update bp3-dark
to bp4-dark
after upgrading the dependencies. 😅 Now I did that and also used styled-components
to override the gray box-shadow
which is used in blueprint 4. So now it should look better.
That looks much better :+1: I have played with it a little and I have found one more bug that will have to be fixed.
Steps to reproduce
- Clear localStorage
- Upload the Image
- Check LS (
savedImage
is set, but nooptions
are set) - Reload the Page
- Options are set to invalid values (only single channel, all values are 0, etc...)
- To actually set
options
you have to manually edit the options
I think the solution is to simplify a bit how options are serialized and saved to localStorage. Instead of using useEffect
and checking optionUpdateAllowed
during rendering cycle, you can implement useLocalStorage
hook that will provide a similar interface as the useState
hook but it will save its state to localStorage. You can find many implementations on the internet, so you can add it in the new file, or if you found a good library add it as a dependency. The only caveat of this is that we are using NextJs, so the page is first server-side rendered. During SSR there is no localStorage available, so the default value will be used and then when the page is hydrated on the client-side it will not match server HTML since it will render differently due to values from localStorage. So you will have to find the implementation of useLocalStorage
that can handle that.
Thank you for pointing out that bug 👍
I did what you said (I found use-local-storage-state
, which explicitly supports SSR and works great for this purpose). Now everything (at least according to my testing) should work correctly.
Looks much better :+1: , one more question. Wouldn't it be possible to get rid of the optionUpdateAllowed
? Because it is kinda anti-pattern in react. What would happen if you used useLocalStorage
also for image
state? Since localStorage is synchronous I think it should load the image from localStorage during the first render so there will be no intermediate state where the page is rendered but the image was not yet loaded.
I did try that, but unfortunately it still resets the options for some reason