-
Notifications
You must be signed in to change notification settings - Fork 69
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 WindowInfo.containerSize
#785
Conversation
compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/WindowInfo.skiko.kt
Show resolved
Hide resolved
compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/WindowInfo.skiko.kt
Outdated
Show resolved
Hide resolved
compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/ComposeBridge.desktop.kt
Outdated
Show resolved
Hide resolved
Consider adding a (skiko) unit test. |
Maybe also add |
compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/WindowInfo.skiko.kt
Outdated
Show resolved
Hide resolved
compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/ComposeBridge.desktop.kt
Outdated
Show resolved
Hide resolved
38000c9
to
6dda784
Compare
b2bffb2
to
f2b7055
Compare
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.
Let's wait for @m-sasha approval
compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/ComposeBridge.desktop.kt
Outdated
Show resolved
Hide resolved
|
||
assertThat(window.scene.platform.windowInfo.containerSize) | ||
.isEqualTo(IntSize( | ||
width = (234 * window.density.density).toInt(), | ||
height = (345 * window.density.density).toInt(), | ||
)) |
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.
I think it's better to test WindowInfo
separately from other things. A good test should validate one thing.
Also, don't you want to test it inside the composition (i.e. with LocalWindow.current.containerSize
?
Fixes a CI fail: https://teamcity.jetbrains.com/buildConfiguration/JetBrainsPublicProjects_Compose_Publish_2_AllGitHubRelease/4313900?buildTab=dependencies&mode=list&state=failed&type=snapshot&hideProblemsFromDependencies=false&hideTestsFromDependencies=false&expandBuildProblemsSection=true&showLog=4313895_245_235.297&logFilter=debug&logView=flowAware#4313895 As it was before #785 Also: - add default getters for source compatibility - add JvmDefaultWithCompatibility to keep binary compatibility on JVM ## Test ``` import androidx.compose.ui.platform.LocalWindowInfo import androidx.compose.ui.window.singleWindowApplication fun main() = singleWindowApplication { val modifiers = LocalWindowInfo.current.keyboardModifiers } ``` This shouldn't require OptIn
Fixes a CI fail: https://teamcity.jetbrains.com/buildConfiguration/JetBrainsPublicProjects_Compose_Publish_2_AllGitHubRelease/4313900?buildTab=dependencies&mode=list&state=failed&type=snapshot&hideProblemsFromDependencies=false&hideTestsFromDependencies=false&expandBuildProblemsSection=true&showLog=4313895_245_235.297&logFilter=debug&logView=flowAware#4313895 As it was before #785 Also: - add default getters for source compatibility - add JvmDefaultWithCompatibility to keep binary compatibility on JVM ## Testing ``` import androidx.compose.ui.platform.LocalWindowInfo import androidx.compose.ui.window.singleWindowApplication fun main() = singleWindowApplication { val modifiers = LocalWindowInfo.current.keyboardModifiers } ``` This shouldn't require OptIn
Fixes a CI fail: https://teamcity.jetbrains.com/buildConfiguration/JetBrainsPublicProjects_Compose_Publish_2_AllGitHubRelease/4313900?buildTab=dependencies&mode=list&state=failed&type=snapshot&hideProblemsFromDependencies=false&hideTestsFromDependencies=false&expandBuildProblemsSection=true&showLog=4313895_245_235.297&logFilter=debug&logView=flowAware#4313895 As it was before #785 Also: - add default getters for source compatibility - add JvmDefaultWithCompatibility to keep binary compatibility on JVM ## Testing ``` import androidx.compose.ui.platform.LocalWindowInfo import androidx.compose.ui.window.singleWindowApplication fun main() = singleWindowApplication { val modifiers = LocalWindowInfo.current.keyboardModifiers } ``` This shouldn't require OptIn
* Add WindowInfo.size * Address feedback * Use delegated property in WindowInfoImpl * Revert size rounding * Rename to containerSize * Add windowInfo.containerSize checks * Address feedback * Provide LocalWindowInfo in tests
#818) Fixes a CI fail: https://teamcity.jetbrains.com/buildConfiguration/JetBrainsPublicProjects_Compose_Publish_2_AllGitHubRelease/4313900?buildTab=dependencies&mode=list&state=failed&type=snapshot&hideProblemsFromDependencies=false&hideTestsFromDependencies=false&expandBuildProblemsSection=true&showLog=4313895_245_235.297&logFilter=debug&logView=flowAware#4313895 As it was before #785 Also: - add default getters for source compatibility - add JvmDefaultWithCompatibility to keep binary compatibility on JVM ## Testing ``` import androidx.compose.ui.platform.LocalWindowInfo import androidx.compose.ui.window.singleWindowApplication fun main() = singleWindowApplication { val modifiers = LocalWindowInfo.current.keyboardModifiers } ``` This shouldn't require OptIn
…rdModifiers (#818) Fixes a CI fail: https://teamcity.jetbrains.com/buildConfiguration/JetBrainsPublicProjects_Compose_Publish_2_AllGitHubRelease/4313900?buildTab=dependencies&mode=list&state=failed&type=snapshot&hideProblemsFromDependencies=false&hideTestsFromDependencies=false&expandBuildProblemsSection=true&showLog=4313895_245_235.297&logFilter=debug&logView=flowAware#4313895 As it was before #785 Also: - add default getters for source compatibility - add JvmDefaultWithCompatibility to keep binary compatibility on JVM ## Testing ``` import androidx.compose.ui.platform.LocalWindowInfo import androidx.compose.ui.window.singleWindowApplication fun main() = singleWindowApplication { val modifiers = LocalWindowInfo.current.keyboardModifiers } ``` This shouldn't require OptIn
Proposed Changes
WindowInfo.containerSize
public APIIt requires for max available space calculation in some material components.
Testing
Test: Try to get window size via
LocalWindowInfo.current