Conversation
priscilawebdev
force-pushed
the
feat/add-dark-mode
branch
from
5826b77 to
4d10a19
Compare
priscilawebdev
force-pushed
the
feat/add-dark-mode
branch
from
4d10a19 to
26b484e
Compare
priscilawebdev
changed the title
Codecov Report
@@ Coverage Diff @@
## master #446 +/- ##
==========================================
- Coverage 87.55% 85.47% -2.08%
==========================================
Files 140 141 +1
Lines 988 1019 +31
Branches 216 210 -6
==========================================
+ Hits 865 871 +6
- Misses 111 122 +11
- Partials 12 26 +14
|
src/design-tokens/ThemeProvider.tsx
Outdated
| </EmotionThemeProvider> | ||
| ); | ||
| const ThemeProvider: React.FC = ({ children }) => { | ||
| const [isDarkMode, setIsDarkMode] = useState(); |
There was a problem hiding this comment.
If users want to have dark mode by default, how we can achieve that right now? I think this might be the entry point using UI_OPTIONS as we did with language, by default would be the default theme.
juanpicado
left a comment
juanpicado
left a comment
There was a problem hiding this comment.
Great job, CSS styling is really nice touched. I'll keep digging and check what else I can do to pass some props from backend to this PR.
types/index.ts
Outdated
| base: string; | ||
| language?: string; | ||
| darkMode?: boolean; | ||
| dark_mode?: boolean; |
There was a problem hiding this comment.
I prefer cammelCase, hopefully we can remove some day all the _ ...
There was a problem hiding this comment.
me too, but then I noticed that we have url_prefix and I decided to do like this
There was a problem hiding this comment.
url_prefix is from the sinopia era, the previous author was quite all over the place using that, it took me few years to get rid of those, but some configurations are still using _ due backward compatibility unfortunately.
src/design-tokens/ThemeProvider.tsx
Outdated
| ); | ||
| const ThemeProvider: React.FC = ({ children }) => { | ||
| const isDarkModeDefault = window?.__VERDACCIO_BASENAME_UI_OPTIONS?.dark_mode; | ||
| const [isDarkMode, setIsDarkMode] = useState(!!isDarkModeDefault); |
There was a problem hiding this comment.
This works really well, but we might use the local storage to persist in the dark mode. Let's imagine the following scenario:
- Admin set
dark_modeastrue - Dark mode is set by default for all users.
- But here is one which likes green or primary color.
- This user will try to persist primary theme but it is not possible right now, on refresh it will switch again.
There was a problem hiding this comment.
Before approving, few things need to be done first:
- Update documentation here https://github.com/verdaccio/website/blob/master/docs/web.md (explain here the dark mode will override
primary_color. - Use
darkModeinsteaddar_modeas configuration.
I tested, all looks good, only small detail with links. Feel free to fix it here or new PR.
|
Kudos, SonarCloud Quality Gate passed!
|
|
Thanks for your PR, the @verdaccio/ui package will be accessible from here for testing purposes: |
|
@priscilawebdev updated https://github.com/verdaccio/website/blob/master/docs/web.md#configuration feel free to merge :) |
3 participants
Type: Feature
Description: Added Dark Mode Theme
Step 01 of https://github.com/verdaccio/ui/issues/427