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

Press0 1328 default to light mode #565

Merged
merged 11 commits into from
May 27, 2024

Conversation

ajayadav09
Copy link
Contributor

@ajayadav09 ajayadav09 commented May 20, 2024

Proposed changes

Default to light mode unless the user has prefer-color-scheme as dark mode.

Type of Change

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • Linting and tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

@ajayadav09 ajayadav09 self-assigned this May 20, 2024
@ajayadav09 ajayadav09 requested a review from arunshenoy99 May 21, 2024 06:36
@ajayadav09 ajayadav09 added the Code Review The PR is in Code Review label May 21, 2024
Copy link
Member

@arunshenoy99 arunshenoy99 left a comment

Choose a reason for hiding this comment

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

@ajayadav09, can we explore using useEffect instead of an event listener, and can we fix the failing tests? Thanks!

@ajayadav09
Copy link
Contributor Author

@ajayadav09, can we explore using useEffect instead of an event listener, and can we fix the failing tests? Thanks!

@arunshenoy99 I tried the useEffect approach to check for changes but the window.matchMedia is a function provided by the window object and it does not change between renders. So to check for changes will have to add listeners to matchMedia.

@ajayadav09 ajayadav09 added QA This PR is in QA and removed Code Review The PR is in Code Review labels May 22, 2024
@arunshenoy99 arunshenoy99 added Ready to merge The Code Review and QA is done and it can be merged. and removed QA This PR is in QA labels May 24, 2024
@arunshenoy99 arunshenoy99 merged commit 92c1f57 into trunk May 27, 2024
3 of 5 checks passed
@arunshenoy99 arunshenoy99 deleted the PRESS0-1328-default-to-light-mode branch May 27, 2024 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready to merge The Code Review and QA is done and it can be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants