Closed ItsJonQ closed 3 years ago
vercel[bot]
commented
3 years ago This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.
🔍 Inspect: https://vercel.com/itsjonq/g2/pvk7kjj0w
✅ Preview: https://g2-git-refactor-color-zustand-removal-2.itsjonq.vercel.app
ItsJonQ
commented
3 years ago @saramarcondes Thanks for the initial review! I'll do my best to type/comment things 🙏
ItsJonQ
commented
3 years ago @saramarcondes I tried! Haha
https://github.com/ItsJonQ/g2/pull/235/commits/c88f4984523faf87953e2250ec9a25c456b531c2
This is a branch for @saramarcondes 's PR https://github.com/ItsJonQ/g2/pull/226
We noticed some state update issues for the initial refactor attempt.
The prop/state/update flow has been simplified and refactored to match the conventions from: https://github.com/ItsJonQ/g2/pull/232
Ultimately, I think it came down to how the color was being passed down into the ColorPicker library.
There was something about the tinycolor2 transformed rgba string that was (sometimes) 1 value off from the one outputted from the RgbaStringColorPicker from react-color.
Because of that inconsistency, they keep trying to update each other, therefore, causing the loop.
Still don't know why there would be an inconsistency? Maybe something to do with color algorithms... I don't know https://github.com/omgovich/react-colorful/blob/master/src/utils/convert.ts
To fix this, I opted to use the RgbaColorPicker, which takes in
{r, g, b, a}.I don't think this could have happened without @saramarcondes 's initial attempt, coupled with our 🍐 sessions. I drew a lot of insight and ideas from that work which helped the recent TextInput refactors as well as this one.