Skip to content
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

Added Theme Mode: Light Mode, Dark Mode and Auto Mode #91

Closed
wants to merge 6 commits into from
Closed

Added Theme Mode: Light Mode, Dark Mode and Auto Mode #91

wants to merge 6 commits into from

Conversation

angelofan
Copy link
Contributor

@angelofan angelofan commented Oct 11, 2024

For #90 , #16 .

  • Dark mode only detect dark class.
  • Added automatic and manual switching of Theme Mode.
  • Added pointer cursor for button in demo html.
  • Added Dark Mode Compatibility in README.md.
  • Added demo code link.

@angelofan
Copy link
Contributor Author

This pull request is ready to review.

@angelofan angelofan closed this Oct 11, 2024
@hccullen
Copy link
Contributor

Did you intend to close this? I wonder whether adding a parameter to the <pwa-install> would be better as it doesn't depend on some other element's class

@angelofan
Copy link
Contributor Author

angelofan commented Oct 15, 2024

Did you intend to close this?

@hccullen After clarification from the maintainer, I tried to convince myself that this component should be consistent with the browser look and feel. So I closed this pull request.

I wonder whether adding a parameter to the <pwa-install> would be better as it doesn't depend on some other element's class

@hccullen Sure. My suggestion is to add an optional class parameter. That will only monitor the set class value after the given class value.

@khmyznikov Any insights?

@angelofan
Copy link
Contributor Author

@hccullen Maybe I understand something different than what you described. You can open a new issue and describe it in detail.

@khmyznikov
Copy link
Owner

Same thoughts from me, I don't see why it should be implemented

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