profile-readme-generator icon indicating copy to clipboard operation
profile-readme-generator copied to clipboard

✨ feat: add light/dark theme switch

Open Martimex opened this issue 1 year ago • 6 comments
trafficstars

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)

  • [ ] Refactor
  • [x] Feature
  • [ ] Bug Fix
  • [x] Enhancement
  • [ ] Documentation Update

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):

Screenshot1

Before pressing (on hover):

Screenshot2

After pressing (on hover):

Screenshot3


Original commit message:


✨ feat: add light/dark theme switch

Include a new switch button that allows users to
preview their README files for both light and
dark themes (default theme is dark one).

This feature aims to reduce the issue with black
colored icons (mostly devicons / simple icons)
being barely visible on the dark canvas. Also it
is a nice UX enhancement for those users who
simply prefer light theming.

Fixes: #44

Martimex avatar Feb 07 '24 19:02 Martimex

Someone is attempting to deploy a commit to a Personal Account owned by @maurodesouza on Vercel.

@maurodesouza first needs to authorize it.

vercel[bot] avatar Feb 07 '24 19:02 vercel[bot]

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:

  • add a new react context/event handle to manage theme changing;
  • let the switch button together with other actions;
  • use the styled-components features to change the theme;
  • create a light theme file within the styles/theme directory (by duplicating default theme values and modifying only the colors), and export a mapping from there.
// styles/themes/index.ts

import { defaultTheme } from './default';
import { lightTheme } from './light';

const themes = {
  dark: defaultTheme,
  light: lightTheme 
}

export { theme }

maurodesouza avatar Mar 16 '24 01:03 maurodesouza

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.

Martimex avatar Mar 17 '24 14:03 Martimex

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 :

https://github.com/maurodesouza/profile-readme-generator/assets/76244675/f60c3f4f-c204-4060-941b-6ac771c2afb5


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 :

solution

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 :

https://github.com/maurodesouza/profile-readme-generator/assets/76244675/85d85137-556e-4b40-bd8c-a88ed9e2ef50

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.

Martimex avatar Mar 20 '24 16:03 Martimex

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

maurodesouza avatar May 10 '24 17:05 maurodesouza