forked from androidx/androidx
-
Notifications
You must be signed in to change notification settings - Fork 107
Migrate window insets padding modifiers from composed API to InsetsPaddingModifierNode
#2572
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
Merged
Merged
Changes from all commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
353d272
Remove unused imports
svastven 3744760
Make toWindowInsets function internal
svastven cc993aa
Align PlatformWindowInsets safeDrawing and safeContent to Android
svastven 02b0f53
Migrate window insets padding modifiers from composed API to InsetsPa…
svastven f5f62c7
Add test to WindowInsetsPaddingTest
svastven 428f757
Remove unused import
svastven d885be1
Lint
svastven 7903e54
Update compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/Ro…
svastven 308ab48
Update exclude function implementation to use PlatformWindowInsets in…
svastven 6f6912b
Disable useSoftwareKeyboardInset in ModalBottomSheetDialog properties
svastven 1ec3640
Disable useSoftwareKeyboardInset in tests
svastven b506bd6
Remove redundant test
svastven 226c7ef
Add windowInsets parameter to internal skiko compose ui test
svastven 3061bcc
Do not provide windowInsets with LocalPlatformWindowInsets but instea…
svastven 1f3a00b
Provide windowInsets to ui test directly instead of LocalPlatformWind…
svastven d6ec7e7
Remove unused imports
svastven 20adaf1
Revert test changes
svastven 06b723d
Update platform window insets tree propagation
svastven c11ea11
Merge branch 'jb-main' into svastven/migrate-insets-modifiers
svastven 3c8d61e
Remove extra line
svastven 3cf6884
Rename ime padding insets animation tests
svastven e0117d0
Revert removed imports
svastven 98f0d4a
Fix platform insets calculation to use ancestorWindowInsets
svastven c807cfb
Make EmptyPlatformWindowInsets object internal API
svastven 854d8f7
Merge branch 'jb-main' into svastven/migrate-insets-modifiers
svastven 43f11c9
Track changes in insetsGetter and update insets padding node state
svastven 43e9cf7
Update taverseKey of PlatformWindowInsetsProviderNode
svastven 12bdfa0
MR updates
svastven 77b12f7
Unify EmptyPlatformWindowInsets
svastven b8ed6c6
Add TODOs
svastven 2bbfee6
Replace equals operator
svastven 49fe3e0
Dump API
svastven File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,7 @@ import androidx.compose.ui.platform.PlatformContext | |
| import androidx.compose.ui.platform.PlatformDragAndDropManager | ||
| import androidx.compose.ui.platform.PlatformDragAndDropSource | ||
| import androidx.compose.ui.platform.PlatformTextInputMethodRequest | ||
| import androidx.compose.ui.platform.PlatformWindowInsets | ||
| import androidx.compose.ui.platform.WindowInfo | ||
| import androidx.compose.ui.scene.CanvasLayersComposeScene | ||
| import androidx.compose.ui.scene.ComposeScene | ||
|
|
@@ -113,6 +114,7 @@ fun runInternalSkikoComposeUiTest( | |
| runTestContext: CoroutineContext = EmptyCoroutineContext, | ||
| testTimeout: Duration = Duration.INFINITE, | ||
| semanticsOwnerListener: PlatformContext.SemanticsOwnerListener? = null, | ||
| windowInsets: PlatformWindowInsets? = null, | ||
| coroutineDispatcher: TestDispatcher = defaultTestDispatcher(), | ||
| block: suspend SkikoComposeUiTest.() -> Unit | ||
| ): TestResult { | ||
|
|
@@ -125,6 +127,7 @@ fun runInternalSkikoComposeUiTest( | |
| testTimeout = testTimeout, | ||
| density = density, | ||
| semanticsOwnerListener = semanticsOwnerListener, | ||
| windowInsets = windowInsets, | ||
| coroutineDispatcher = coroutineDispatcher, | ||
| ).runTest(block) | ||
| } | ||
|
|
@@ -158,6 +161,7 @@ open class SkikoComposeUiTest @InternalTestApi constructor( | |
| private val testTimeout: Duration = Duration.INFINITE, | ||
| override val density: Density = Density(1f), | ||
| private val semanticsOwnerListener: PlatformContext.SemanticsOwnerListener?, | ||
| private val windowInsets: PlatformWindowInsets?, | ||
| private val coroutineDispatcher: TestDispatcher = defaultTestDispatcher(), | ||
| ) : ComposeUiTest { | ||
| init { | ||
|
|
@@ -178,6 +182,7 @@ open class SkikoComposeUiTest @InternalTestApi constructor( | |
| effectContext = effectContext, | ||
| density = density, | ||
| semanticsOwnerListener = null, | ||
| windowInsets = null, | ||
| ) | ||
|
|
||
| constructor( | ||
|
|
@@ -195,6 +200,7 @@ open class SkikoComposeUiTest @InternalTestApi constructor( | |
| testTimeout = testTimeout, | ||
| density = density, | ||
| semanticsOwnerListener = null, | ||
| windowInsets = null, | ||
| ) | ||
|
|
||
| private val composeRootRegistry = ComposeRootRegistry() | ||
|
|
@@ -520,6 +526,8 @@ open class SkikoComposeUiTest @InternalTestApi constructor( | |
|
|
||
| override val semanticsOwnerListener: PlatformContext.SemanticsOwnerListener? | ||
| get() = [email protected] | ||
| override val windowInsets: PlatformWindowInsets | ||
| get() = [email protected] ?: super.windowInsets | ||
|
|
||
| override val dragAndDropManager: PlatformDragAndDropManager = TestDragAndDropManager() | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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'm not sure that it's the right way to expose it to public (under annotation but anyway) testing API.
In general, I'd keep this out of scope for now. Our own tests have a number of examples of how to provide
PlatformContextinside testsThere 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.
It should only be as part of the internal API since the constructor is marked as internal API and also it can only be provided to the
runInternalSkikoComposeUiTestwhich is marked internal. I followed similar approach as with thesemanticsOwnerListenerabove, which can also be provided only in terms of the internal API.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.
Well, adding it here indeed doesn't mean that we'll need to support compatiblity forever.
But I still think that the existing pattern (an arg above) is not the right way to expose it.
We might change it in the future, so I won't block PR because of it