Skip to content

support light and dark #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

support light and dark #40

wants to merge 14 commits into from

Conversation

JerryWu1234
Copy link
Collaborator

@JerryWu1234 JerryWu1234 commented May 24, 2025

image
image

JerryWu1234 and others added 7 commits May 22, 2025 15:48
…uration

Simplify the global CSS and Tailwind configuration by removing unused variables and consolidating theme-related settings. This improves maintainability and reduces redundancy in the codebase.
The host, port, and allowedHosts configurations were removed from the dev server settings to simplify the configuration and rely on Vite's default behavior. This change reduces unnecessary customization and aligns with standard Vite practices.
Copy link

changeset-bot bot commented May 24, 2025

⚠️ No Changeset found

Latest commit: 437278b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Member

@wmertens wmertens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JerryWu1234 you can do it without JS, see the theme toggle in Qwik docs, it uses only css

But there's still a visible tasks in docs to pass the theme to the editor, we'll get that out later

@JerryWu1234
Copy link
Collaborator Author

@JerryWu1234 you can do it without JS, see the theme toggle in Qwik docs, it uses only css

But there's still a visible tasks in docs to pass the theme to the editor, we'll get that out later

there's still a visible tasks in docs to pass the theme to the editor,

I can try it.

@JerryWu1234 you can do it without JS, see the theme toggle in Qwik docs, it uses only css

I checked branch main and build/v2.
here is that the themeToggle component is for switching theme.
SO I'm confused about what you meant by implementing it using only CSS, without JavaScript

@wmertens

https://github.com/JerryWu1234/qwik/blob/765ce4a7e7399bbe56e52da85775b5f7e2b4c6a4/packages/docs/src/components/theme-toggle/theme-toggle.tsx#L44

@wmertens
Copy link
Member

wmertens commented May 27, 2025

@JerryWu1234 ah sorry, I didn't finish this :(

see QwikDev/qwik@bce2f5a

You can see how I change the text of the button using only CSS, and try to remove the store anywhere. Anyway, most of the themed things in the docs are CSS-only, they don't need JS to show the right thing. And the toggle can just be a sync$ function that adds the correct class on <html>.

Ideally, there are 3 states: light (sets data-theme=light), dark (sets data-theme=dark) and system (removes data-theme).

and then everything is done with css variables, like

:root {
  --background: #ffffff;
  --text-color: #000000;
}

/* Dark theme override */
html[data-theme="dark"] {
  --background: #1a1a1a;
  --text-color: #ffffff;
}

/* Respect system preference if no data-theme is set */
@media (prefers-color-scheme: dark) {
  :root:not([data-theme]) {
    --background: #1a1a1a;
    --text-color: #ffffff;
  }
}

- Add theme script for initial theme detection and application
- Update global.css with theme variables and dark mode selector
- Refactor ThemeToggle component to use localStorage for theme persistence
- Modify vite config to handle build-specific alias configuration
- Add type definitions path to package.json
```

The commit message follows the guidelines by:
1. Using "feat" type since it introduces new theme functionality
2. Including "(theme)" scope as it's clearly theme-related
3. Keeping description under 50 chars and starting with lowercase
4. Adding a body that summarizes the key changes without repeating the subject
5. Using imperative mood throughout
6. Focusing on the significant functional changes rather than every detail
@JerryWu1234 JerryWu1234 marked this pull request as draft May 28, 2025 07:14
- Normalize quotes from single to double quotes
- Fix missing semicolons and newlines
- Reorder tailwind classes consistently
- Remove unused useDark hook
- Update theme script to be more reliable
- Apply prettier formatting across the codebase
- Remove ThemeScript component and inline theme script logic in devtools
- Remove unused useDark hook from ThemeToggle
- Simplify theme toggle logic by removing dark state check
…ity condition

- Remove unused ThemeScript import to clean up dependencies
- Fix devtools panel visibility to only show when state.isOpen.value is true
@JerryWu1234 JerryWu1234 marked this pull request as ready for review June 3, 2025 05:21
Update all double quotes to single quotes across the codebase for consistency. Added 'singleQuote: true' to prettier configuration to enforce this style. This change improves code style consistency and aligns with the project's linting rules.
@JerryWu1234 JerryWu1234 marked this pull request as draft June 3, 2025 07:26
@JerryWu1234
Copy link
Collaborator Author

JerryWu1234 commented Jun 3, 2025

@JerryWu1234

  • 1. LIght and dark have a bug when switching theme

- Rename theme storage key for compatibility with vite-plugin-inspect
- Extract theme script logic into separate component
- Remove unused imports and simplify ThemeToggle component
- Replace inline script with ThemeScript component in devtools
- Add 'auto' theme option that follows system preference
- Replace button with select dropdown for theme selection
- Improve theme script to handle auto theme case
- Fix extra space in search input styling
@JerryWu1234 JerryWu1234 marked this pull request as ready for review June 4, 2025 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants