-
Notifications
You must be signed in to change notification settings - Fork 187
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
Always follow the desired theme for Pin, Incoming Call and Element Call screens #3165
Conversation
…l theme set by the user.
…he ElementTheme block. Closes #3153
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3165 +/- ##
========================================
Coverage 76.11% 76.11%
========================================
Files 1640 1641 +1
Lines 38622 38622
Branches 7469 7471 +2
========================================
+ Hits 29396 29397 +1
Misses 5333 5333
+ Partials 3893 3892 -1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LGTM!
@jmartinesp I have added another commit, can you double check please? Also I have found an issue about assets stored in |
*/ | ||
@Composable | ||
fun ElementThemeApp( | ||
appPreferencesStore: AppPreferencesStore, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having the design system library depending on the preferences one is a bit weird to be honest. Maybe this should be in a separate module that depends on both? Although that also sounds like an overkill.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's not a big deal to me, and it's better than duplicated code. Also in the future (EXE) it's better to centralize where the application theme is configured.
|
Content
Fix several issue around theming: Pin and Incoming call was not using the application setting for theme.
ElementCall was reading the current theme but before it was set.
Motivation and context
Dark theme everywhere when selected manually by the user or by following the system setting.
Closes #3153
Screenshots / GIFs
Tests
I was confused by the icon change in the screenshot at #3118 , but now the icons are always the same, which is also a bit confusing. But this is on the web part.
Tested devices
Checklist