Dynamic iOS and Web color theme #714
Conversation
alexzhirkevich
changed the title
compose/mpp/demo/src/commonMain/kotlin/androidx/compose/mpp/demo/App.kt
Outdated
Show resolved
Hide resolved
| /** | ||
| * Composition local for [UITraitCollection] | ||
| */ | ||
| val LocalUITraitCollectionState = staticCompositionLocalOf<State<UITraitCollection>> { |
There was a problem hiding this comment.
I wouldn't expose platform.UIKit.* to public compose API
There was a problem hiding this comment.
LocalUIViewController is already exposed 👉👈
There was a problem hiding this comment.
Right... 🤔 and it cannot be internal because of usage from other module
There was a problem hiding this comment.
It can be annotated with @InternalComposeUiApi. But i found it useful because it is the only way of getting an updated value of it without костылей in Compose-only projects
There was a problem hiding this comment.
This local and uiviewcontroller local too will be useful for community library developers. Not hide it by internal please.
View controller was changed to public by my request to implement valid logic in moko-permissions.
There was a problem hiding this comment.
Maybe it's better to expose just userInterfaceStyle instead of entire collection then?
@dima-avdeev-jb what do you think?
There was a problem hiding this comment.
It also have an interface idiom property that can be useful. Not sure if It can update at runtime
There was a problem hiding this comment.
I think - it is fine to have it as a public val LocalUITraitCollectionState.
Compose for iOS is still in alpha overall - and we can change this decision in the future.
But, maybe annotation @InternalComposeUiApi can also be helpfull. Library authors can also at it to their code.
| return LocalUITraitCollectionState.current.value | ||
| .userInterfaceStyle == UIUserInterfaceStyleDark |
There was a problem hiding this comment.
Added
isSystemInDarkThemesupport for iOS < 13
I fixed a crash on iOS 12 recently and wrote that comment you mentioned. So if we're speaking about current state of support - it's not crashing now 😅
EDIT: System dark mode was introduced in iOS 13, so the current state is fine here
actually i'm not sure if Compose works on iOS < 13
Yes, it does. UIUserInterfaceStyleDark exists since iOS 12, so it should be fine (I'm not totally sure that minimal iOS version is properly documented at the moment)
There was a problem hiding this comment.
UIUserInterfaceStyleDarkexists since iOS 12
It should be a check for iOS 12 here?
There was a problem hiding this comment.
Actually adding support for ios 12 wasn't valuable because dark mode was introduced in iOS 13 😅
There was a problem hiding this comment.
It should be a check for iOS 12 here?
Compose won't work on iOS < 12 because of skia, so this check is not required
|
Hello! Thanks! But, first can you please sign the Google Contributor’s License Agreement at https://cla.developers.google.com/. As a referece, you can look at screenshots in this PR: #699 And can you please post a screenshot of the signed agreement here as a comment? |
|
Yep, i have already signed it |
|
Made 2nd PR with probably better api - #715 - without exposing UIKit and creating different actuals |
|
Thanks! If second approach will works as a current PR on iOS side, - then I think it is better to close current PR. |
Proposed Changes
LocalUITraitCollectionStateto allow observing trait collection changes in compositionisSystemInDarkThemesupport for iOS < 13 (actually i'm not sure if Compose works on iOS < 13 but a nice bonus)Testing
Test: try to switch color theme while running mpp app
Issues Fixed
Fixes: JetBrains/compose-multiplatform#3213