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

Add support for Window::theme on the web #2687

Merged
merged 1 commit into from
Feb 20, 2023
Merged

Add support for Window::theme on the web #2687

merged 1 commit into from
Feb 20, 2023

Conversation

tronical
Copy link
Contributor

  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

@ogoffart
Copy link
Contributor

I think this change is useful as is, so IMHO shouldn't prevent to be merged. But it could also emit the ThemeChanged event when the media query changes (using for example MediaQueryList::set_onchange)

@tronical
Copy link
Contributor Author

I think this change is useful as is, so IMHO shouldn't prevent to be merged. But it could also emit the ThemeChanged event when the media query changes (using for example MediaQueryList::set_onchange)

Winit already implements the part of sending ThemeChanged when targeting the web. It's just missing the getter for the current value.

Copy link
Member

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

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

Only a minor nit with the changelog, otherwise looks fine

CHANGELOG.md Outdated Show resolved Hide resolved
@tronical
Copy link
Contributor Author

Good point, applied:). Feel free to choose merge with squash. (Or I can also force push a squash if you prefer)

@madsmtm
Copy link
Member

madsmtm commented Feb 19, 2023

Don't have a preference for how you do the commits, they'll be squashed once merged anyhow ;)

I'll need you to fix the conflicting changelog though, since I don't have write access to your fork.

@tronical
Copy link
Contributor Author

Oops, not sure why that permission isn't there. I rebased, squashed, and resolved the conflict.

@madsmtm madsmtm merged commit a31f71e into rust-windowing:master Feb 20, 2023
@madsmtm
Copy link
Member

madsmtm commented Feb 20, 2023

Thanks!

@madsmtm madsmtm added DS - web S - platform parity Unintended platform differences labels Feb 20, 2023
@tronical tronical deleted the simon/web-theme-getter branch February 20, 2023 08:20
kchibisov pushed a commit to kchibisov/winit that referenced this pull request Mar 1, 2023
kchibisov pushed a commit that referenced this pull request Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DS - web S - platform parity Unintended platform differences
Development

Successfully merging this pull request may close these issues.

3 participants