Open Martimex opened 7 months ago
Someone is attempting to deploy a commit to a Personal Account owned by @maurodesouza on Vercel.
@maurodesouza first needs to authorize it.
Hi @Martimex
I'm delighted with your contribution! Thanks so much!
Regarding the PR content, I believe switching to a global theme would be more advantageous.
I've requested changes. Please let me know if you can implement them or if you need any more information.
Summary:
// styles/themes/index.ts
import { defaultTheme } from './default';
import { lightTheme } from './light';
const themes = {
dark: defaultTheme,
light: lightTheme
}
export { theme }
Thank you so much for the detailed insight in this PR !
I've requested changes. Please let me know if you can implement them or if you need any more information.
I am definitely going to apply requested changes to the branch and add an updated PR soon, probably within few days time.
Hey @maurodesouza,
I have been trying to implement the changes. So far, the theme change functionality is completed and it works nicely within the whole app.
The theme change functionality is implemented by using APP_SET_THEME synthetic event, that finally leads to this function (in _app.tsx
) :
const [currTheme, setCurrTheme] = useState(themes['dark']);
const handleSetTheme = (e: CustomEvent) => {
setCurrTheme(themes[e.detail]);
}
The currTheme
variable is then provided as a < ThemeProvider > argument, which causes rerender whenever theme changes (also inside _app.tsx
) :
<ThemeProvider theme={currTheme}>
There is one difficult issue, though. Whenever I try to change the theme, this happens :
After some investigation, I found out that the elements with this weird behaviour are ones, which use any transition from Framer Motion. By that I mean those two Reorder Lists visible on the video above.
Then, after some trials I came up with only this solution, which minifies (but does not get rid off) the problem :
Left part of the image is code from _app.tsx
; the right part is global.ts
(where the global styles are stored).
So, after adding the code from the image :
the problem is not present if user clicks the button with a brief delay. However, as soon as button is being clicked consecutively around 3 times, then the reflow issue appears again.
Honestly, I think I am running out of ideas on how to deal with this reflow thing properly. Current approach is more like a workaround IMO. Maybe there is an option to trigger theme change in another way, but it's just me cannot figure it out.
Should we try to find a different way to implement the feature then ? Or maybe you have had experienced similiar problem before and you managed to solve it ? Please let me know, I would appreciate any response from you.
Hi @Martimex! Apologies for my lateness!
Could you commit/send me what you have? I can take it from there and finish it up.
You've already done a great job! Thanks a lot
Hello @maurodesouza,
Very sorry for no response lately.
A few days ago I came back again to this issue and got another improvement that I want to share.
Most of the things you asked me to fix are now finished, with the only exception being that glitch I mentioned earlier. Also I found a way to "minify" its' impact, but still it is not perfect :
Left part of the image is code from _app.tsx
; the right part is global.ts
(where the global styles are stored).
I believe the glitch still needs some investigation. It happens to impact Framer Motion Reorder Lists only. Anyways, I wanted to submit this PR, because it contains some other changes requested by you.
In case of any doubts or issues, please let me know. I am more active recently here on Github, so I will be able to respond much quicker.
Hello!
For this feature I tried to apply as little changes to codebase as possible. In case of any doubts / questions about this PR, please let me know. Also I would really appreciate any related feedback. If this PR need some fixes before accepting, I am fine making additional changes to it.
Fixes: #44
What type of PR is this? (check all applicable)
What I did
I have added a new button that makes toggling between canvas themes a smooth experience. The button is responsible for changing the canvas background color and text color. No other canvas appliances (like images, icons, etc.) are affected. By hovering over it, user can see a short message of what would happen after pressing the switch.
Take a look at below screenshots to see the feature in action:
Initial state (on load):
Before pressing (on hover):
After pressing (on hover):
Original commit message: