-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
Allow duplicate configurations as an experimental feature #731
Conversation
WalkthroughThe recent updates to the Decompose library introduced several new classes and methods, particularly adding support for handling child keys and experimental flags for duplicate configurations. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Child
participant ExperimentFlags
User ->> App: Initialize App
App ->> ExperimentFlags: Check if duplicateConfigurationEnabled is true
App ->> Child: Get key for new child
Child ->> App: Return child with key
App ->> Child: Add or Update child with key in ChildStack
App ->> User: App running with new Child handling logic
Poem
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (28)
- decompose/api/android/decompose.api (3 hunks)
- decompose/api/decompose.klib.api (4 hunks)
- decompose/api/jvm/decompose.api (3 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/Child.kt (1 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/ChildKey.kt (1 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/DecomposeExperimentFlags.kt (1 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/GettingList.kt (1 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenFactory.kt (1 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenNavigator.kt (7 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/stack/ChildStack.kt (2 hunks)
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenBasicTest.kt (3 hunks)
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenLifecycleTest.kt (2 hunks)
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenRetainedInstanceTest.kt (18 hunks)
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenSavedStateTest.kt (2 hunks)
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenTestBase.kt (4 hunks)
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/stack/ChildStackIntegrationTest.kt (18 hunks)
- decompose/src/webTest/kotlin/com/arkivanov/decompose/router/stack/webhistory/DefaultWebHistoryControllerTest.kt (3 hunks)
- extensions-android/src/main/java/com/arkivanov/decompose/extensions/android/stack/StackRouterView.kt (5 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/Pages.kt (3 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/Children.kt (4 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/AbstractStackAnimation.kt (4 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/MovableStackAnimation.kt (1 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/SimpleStackAnimation.kt (1 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/predictiveback/PredictiveBackAnimation.kt (2 hunks)
- extensions-compose/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/stack/ChildrenTest.kt (5 hunks)
- sample/shared/compose/src/commonMain/kotlin/com/arkivanov/sample/shared/cards/CardsContent.kt (6 hunks)
- sample/shared/compose/src/commonMain/kotlin/com/arkivanov/sample/shared/customnavigation/CustomNavigationContent.kt (1 hunks)
- sample/shared/compose/src/commonMain/kotlin/com/arkivanov/sample/shared/multipane/MultiPaneContent.kt (2 hunks)
Files skipped from review due to trivial changes (2)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/DecomposeExperimentFlags.kt
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenRetainedInstanceTest.kt
Additional comments not posted (91)
decompose/src/commonMain/kotlin/com/arkivanov/decompose/GettingList.kt (1)
12-13
: LGTM! But verify the usage of inline and crossinline keywords.The
mapped
function is correct. Ensure that the inline and crossinline keywords are used appropriately to optimize performance.Verification successful
LGTM! The usage of
inline
andcrossinline
keywords is appropriate.The
mapped
function is correct, and the keywords are used consistently across the codebase.
inline
is used to reduce function call overhead.crossinline
ensures that lambdas do not use non-local returns.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of inline and crossinline keywords in the codebase. # Test: Search for the usage of `inline` and `crossinline` keywords. Expect: Correct usage in all instances. rg --type kotlin 'inline|crossinline'Length of output: 6560
decompose/src/commonMain/kotlin/com/arkivanov/decompose/ChildKey.kt (2)
9-17
: LGTM! But verify the efficiency of the implementation.The
keyed
function is correct. Ensure that the function is efficient and that the use of HashMap is justified.Verification successful
The use of
HashMap
in thekeyed
function is justified and efficient given the context.The
switch
,switchDefault
, andswitchExperimental
functions handle lists ofChildNavState<C>
dynamically, and theHashMap
usage ensures efficient handling of unique configurations.
switchDefault
checks for unique configurations using aHashSet
.switchExperimental
uses thekeyed
function to map states and items efficiently.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the efficiency of the `keyed` function implementation. # Test: Search for the usage of HashMap in the codebase. Expect: Efficient usage in all instances. rg --type kotlin 'HashMap'Length of output: 655
Script:
#!/bin/bash # Description: Find where the `keyed` function is used and inspect the sizes of the lists being processed. # Search for the usage of the `keyed` function in the codebase. rg --type kotlin '\.keyed'Length of output: 808
Script:
#!/bin/bash # Description: Inspect the sizes of the lists passed to the `keyed` function in `ChildrenNavigator.kt`. # Search for list initializations and their sizes in `ChildrenNavigator.kt`. rg --type kotlin 'List' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenNavigator.ktLength of output: 1333
Script:
#!/bin/bash # Description: Inspect the implementations of `switch`, `switchDefault`, and `switchExperimental` functions in `ChildrenNavigator.kt`. # Extract the implementations of the `switch`, `switchDefault`, and `switchExperimental` functions. ast-grep --lang kotlin --pattern $'fun switch($_) { $$$ } fun switchDefault($_) { $$$ } fun switchExperimental($_) { $$$ }' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenNavigator.ktLength of output: 743
Script:
#!/bin/bash # Description: Inspect the implementations of `switch`, `switchDefault`, and `switchExperimental` functions in `ChildrenNavigator.kt`. # Extract the implementation of the `switch` function. ast-grep --lang kotlin --pattern $'fun switch($_) { $$$ }' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenNavigator.kt # Extract the implementation of the `switchDefault` function. ast-grep --lang kotlin --pattern $'fun switchDefault($_) { $$$ }' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenNavigator.kt # Extract the implementation of the `switchExperimental` function. ast-grep --lang kotlin --pattern $'fun switchExperimental($_) { $$$ }' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenNavigator.ktLength of output: 483
Script:
#!/bin/bash # Description: Inspect the implementations of `switch`, `switchDefault`, and `switchExperimental` functions in `ChildrenNavigator.kt`. # Search for the `switch` function definition. rg --type kotlin 'fun switch' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenNavigator.kt # Search for the `switchDefault` function definition. rg --type kotlin 'fun switchDefault' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenNavigator.kt # Search for the `switchExperimental` function definition. rg --type kotlin 'fun switchExperimental' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenNavigator.ktLength of output: 731
Script:
#!/bin/bash # Description: Extract the full implementations of `switch`, `switchDefault`, and `switchExperimental` functions in `ChildrenNavigator.kt`. # Extract the full implementation of the `switch` function. rg --no-heading -A 20 'fun switch' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenNavigator.kt # Extract the full implementation of the `switchDefault` function. rg --no-heading -A 20 'fun switchDefault' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenNavigator.kt # Extract the full implementation of the `switchExperimental` function. rg --no-heading -A 20 'fun switchExperimental' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenNavigator.ktLength of output: 4695
5-7
: LGTM! But verify the usage of the experimental annotation.The
ChildKey
class is correct. Ensure that the experimental annotation is used appropriately and that the value class is necessary.Verification successful
LGTM! The usage of the
@ExperimentalDecomposeApi
annotation is appropriate.The
ChildKey
class and other instances where the@ExperimentalDecomposeApi
annotation is used are correctly marked as experimental features.
decompose/src/commonMain/kotlin/com/arkivanov/decompose/ChildKey.kt
sample/shared/shared/src/androidMain/kotlin/com/arkivanov/sample/shared/tabs/TabsView.kt
sample/shared/shared/src/androidMain/kotlin/com/arkivanov/sample/shared/root/RootView.kt
sample/shared/shared/src/androidMain/kotlin/com/arkivanov/sample/shared/root/NotImplementedView.kt
sample/shared/shared/src/androidMain/kotlin/com/arkivanov/sample/shared/Transitions.kt
sample/shared/shared/src/androidMain/kotlin/com/arkivanov/sample/shared/counters/CountersView.kt
sample/shared/shared/src/androidMain/kotlin/com/arkivanov/sample/shared/counters/counter/CounterView.kt
extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/predictiveback/PredictiveBackAnimatable.kt
extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/predictiveback/MaterialPredictiveBackAnimatable.kt
extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/predictiveback/PredictiveBackAnimation.kt
extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/Pages.kt
extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/PagesScrollAnimation.kt
extensions-compose/src/nonAndroidMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/predictiveback/PredictiveBackGestureIcon.kt
extensions-compose/src/nonAndroidMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/predictiveback/PredictiveBackGestureOverlay.kt
extensions-android/src/main/java/com/arkivanov/decompose/extensions/android/ViewContext.kt
extensions-android/src/main/java/com/arkivanov/decompose/extensions/android/ViewContextExt.kt
extensions-android/src/main/java/com/arkivanov/decompose/extensions/android/stack/StackRouterView.kt
extensions-android/src/main/java/com/arkivanov/decompose/extensions/android/DefaultViewContext.kt
decompose/src/webMain/kotlin/com/arkivanov/decompose/router/stack/webhistory/DefaultWebHistoryController.kt
decompose/src/commonMain/kotlin/com/arkivanov/decompose/DecomposeExperimentFlags.kt
decompose/src/commonMain/kotlin/com/arkivanov/decompose/Child.kt
decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/Pages.kt
decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/ChildPages.kt
decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/PagesNavigator.kt
decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/PagesNavigation.kt
decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/PagesNavigatorExt.kt
decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/stack/ChildStack.kt
decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/stack/webhistory/WebHistoryController.kt
decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/ChildPagesFactory.kt
decompose/src/androidMain/kotlin/com/arkivanov/decompose/DeeplinkUtils.kt
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the experimental annotation in the codebase. # Test: Search for the usage of `@ExperimentalDecomposeApi` annotation. Expect: Correct usage in all instances. rg --type kotlin '@ExperimentalDecomposeApi'Length of output: 6791
Script:
#!/bin/bash # Description: Check the context of the `@ExperimentalDecomposeApi` annotation in the `ChildKey` class and other relevant files to ensure proper usage. # Search for the `@ExperimentalDecomposeApi` annotation in the `ChildKey` class file and show surrounding lines for context. rg --type kotlin --context 5 '@ExperimentalDecomposeApi' decompose/src/commonMain/kotlin/com/arkivanov/decompose/ChildKey.kt # Search for the `@ExperimentalDecomposeApi` annotation in other relevant files and show surrounding lines for context. rg --type kotlin --context 5 '@ExperimentalDecomposeApi' sample/shared/shared/src/androidMain/kotlin/com/arkivanov/sample/shared/tabs/TabsView.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' sample/shared/shared/src/androidMain/kotlin/com/arkivanov/sample/shared/root/RootView.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' sample/shared/shared/src/androidMain/kotlin/com/arkivanov/sample/shared/root/NotImplementedView.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' sample/shared/shared/src/androidMain/kotlin/com/arkivanov/sample/shared/Transitions.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' sample/shared/shared/src/androidMain/kotlin/com/arkivanov/sample/shared/counters/CountersView.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' sample/shared/shared/src/androidMain/kotlin/com/arkivanov/sample/shared/counters/counter/CounterView.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/predictiveback/PredictiveBackAnimatable.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/predictiveback/MaterialPredictiveBackAnimatable.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/predictiveback/PredictiveBackAnimation.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/Pages.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/PagesScrollAnimation.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' extensions-compose/src/nonAndroidMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/predictiveback/PredictiveBackGestureIcon.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' extensions-compose/src/nonAndroidMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/predictiveback/PredictiveBackGestureOverlay.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' extensions-android/src/main/java/com/arkivanov/decompose/extensions/android/ViewContext.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' extensions-android/src/main/java/com/arkivanov/decompose/extensions/android/ViewContextExt.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' extensions-android/src/main/java/com/arkivanov/decompose/extensions/android/stack/StackRouterView.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' extensions-android/src/main/java/com/arkivanov/decompose/extensions/android/DefaultViewContext.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' decompose/src/webMain/kotlin/com/arkivanov/decompose/router/stack/webhistory/DefaultWebHistoryController.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' decompose/src/commonMain/kotlin/com/arkivanov/decompose/DecomposeExperimentFlags.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' decompose/src/commonMain/kotlin/com/arkivanov/decompose/Child.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/Pages.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/ChildPages.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/PagesNavigator.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/PagesNavigation.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/PagesNavigatorExt.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/stack/ChildStack.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/stack/webhistory/WebHistoryController.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/ChildPagesFactory.kt rg --type kotlin --context 5 '@ExperimentalDecomposeApi' decompose/src/androidMain/kotlin/com/arkivanov/decompose/DeeplinkUtils.ktLength of output: 22771
extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/SimpleStackAnimation.kt (1)
19-19
: LGTM! But verify the usage of theremember
composable.The
Child
function is correct. Ensure that theremember
composable is used appropriately and that the key is necessary.extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/MovableStackAnimation.kt (1)
21-21
: LGTM! But verify the usage of theremember
composable.The
Child
function is correct. Ensure that theremember
composable is used appropriately and that the keys and direction are necessary.Verification successful
Verified the usage of the
remember
composable.The
remember
composable is used appropriately in theMovableStackAnimation.kt
file and other instances across the codebase. The keys and direction parameters are necessary to ensure the correct state is remembered for each unique combination.
MovableStackAnimation.kt
:
- Line 21:
remember(item.child.key, item.otherChild?.key, item.direction)
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `remember` composable in the codebase. # Test: Search for the usage of `remember` composable. Expect: Correct usage in all instances. rg --type kotlin 'remember'Length of output: 11870
decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/stack/ChildStack.kt (2)
22-27
: LGTM!The constructor correctly initializes the
ChildStack
with the providedconfiguration
,instance
, andkey
.
34-38
: LGTM!The constructor correctly initializes the
ChildStack
with aChildKey
derived from theconfiguration
.decompose/src/commonMain/kotlin/com/arkivanov/decompose/Child.kt (3)
8-9
: LGTM!The
key
property is correctly defined and marked as experimental.
11-27
: LGTM!The constructor and copy method correctly include the
key
property, ensuring each created child has a unique key.
29-44
: LGTM!The constructor and copy method correctly include the
key
property, ensuring each destroyed child has a unique key.extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/Children.kt (4)
Line range hint
19-35
: LGTM!The
Children
composable function correctly useskey
for state handling and is appropriately annotated with@OptIn(ExperimentalDecomposeApi::class)
.
Line range hint
43-54
: LGTM!The second
Children
composable function correctly subscribes to the state and passes it to the firstChildren
function.
58-60
: LGTM!The
getKeys
function correctly maps items to their hashed keys and is appropriately annotated with@OptIn(ExperimentalDecomposeApi::class)
.
Line range hint
63-73
: LGTM!The
retainStates
function correctly retains states based on the current keys.sample/shared/compose/src/commonMain/kotlin/com/arkivanov/sample/shared/multipane/MultiPaneContent.kt (2)
50-51
: LGTM!The
MultiPaneContent
composable function correctly useskey
for state handling inmovableContentOf
andSaveableStateProvider
.
Line range hint
76-92
: LGTM!The
OldDetailsKeyRemoved
composable function correctly removes the state of the old details key when it changes.extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/AbstractStackAnimation.kt (2)
Line range hint
77-93
: LGTM! But verify the function usage in the codebase.The code changes are approved. Ensure that all function calls to
getAnimationItems
match the new key-based approach.Verification successful
Verified function usage: The function
getAnimationItems
is correctly utilized within the same file, ensuring the new key-based approach is consistently applied.
extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/AbstractStackAnimation.kt
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `getAnimationItems` match the new key-based approach. # Test: Search for the function usage. Expect: Only occurrences of the new key-based approach. rg --type kotlin $'getAnimationItems(newStack: ChildStack<C, T>, oldStack: ChildStack<C, T>?)'Length of output: 95
Script:
#!/bin/bash # Description: Search for all occurrences of `getAnimationItems` to verify its usage and ensure it matches the new key-based approach. # Test: Search for all occurrences of `getAnimationItems` in the codebase and display the surrounding lines for context. rg --type kotlin 'getAnimationItems' -A 5 -B 5Length of output: 4555
32-32
: LGTM! But verify the function usage in the codebase.The code changes are approved. Ensure that all function calls to
invoke
match the new key-based approach.extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/Pages.kt (1)
36-36
: LGTM! But verify the function usage in the codebase.The code changes are approved. Ensure that all function calls to
Pages
match the new key-based approach.decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenTestBase.kt (1)
40-42
: LGTM! But verify the function usage in the codebase.The code changes are approved. Ensure that all function calls to
after
match the new flag reset approach.extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/predictiveback/PredictiveBackAnimation.kt (3)
16-16
: Import statement is correct.The import of
ChildKey
is necessary for the changes in this file.
70-71
: LGTM!The initialization of
activeKeys
withremember
and the setup ofhandler
are correct and ensure proper state management.
78-83
: LGTM!The use of
key(child.key)
and the management ofactiveKeys
withinDisposableEffect
are correct and ensure proper state tracking.sample/shared/compose/src/commonMain/kotlin/com/arkivanov/sample/shared/customnavigation/CustomNavigationContent.kt (1)
119-119
: LGTM!The use of
key(child.key)
ensures that each child is uniquely identified, which is important for state management in Compose.sample/shared/compose/src/commonMain/kotlin/com/arkivanov/sample/shared/cards/CardsContent.kt (7)
50-50
: Import statement is correct.The import of
ChildKey
is necessary for the changes in this file.
102-104
: LGTM!The mapping of
items
to includekey
is correct and ensures each child is uniquely identified.
123-124
: LGTM!The use of
key(key)
ensures that each child is uniquely identified, which is important for state management in Compose.
145-145
: LGTM!The filtering of
lastItems
based onkey
is correct and ensures accurate state management.
156-165
: LGTM!The use of
key
in thediff
function is correct and ensures that the items are uniquely identified and managed.
272-272
: LGTM!The inclusion of
key
as a property in theItem
data class is correct and necessary for uniquely identifying each item.
Line range hint
246-248
: LGTM!The definition of
KEY_A1
andKEY_A2
asChildKey
instances is appropriate and ensures consistent key management.extensions-compose/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/stack/ChildrenTest.kt (14)
17-19
: Import statements are correct.The imports of
ChildKey
andExperimentalDecomposeApi
are necessary for the changes in this file.
32-32
: LGTM!The use of
@OptIn(ExperimentalDecomposeApi::class)
is correct and necessary for using experimental features.
44-44
: LGTM!The test correctly verifies that the active child is displayed when there is no back stack.
53-53
: LGTM!The test correctly verifies that child B is displayed when pushed after child A.
56-56
: LGTM!The test correctly verifies that child B is displayed when pushed after child A.
61-69
: LGTM!The test correctly verifies that child A2 is displayed when pushed after child A1.
73-77
: LGTM!The test correctly verifies that child A is displayed when child B is popped.
82-92
: LGTM!The test correctly verifies that child A1 is displayed when child A2 is popped.
96-116
: LGTM!The test correctly verifies that the state is restored for child A when child B is popped.
120-144
: LGTM!The test correctly verifies that the state is not restored for child B when it is pushed again after being popped.
148-151
: LGTM!The test correctly verifies that child A is disposed when child B is pushed.
156-164
: LGTM!The test correctly verifies that child A1 is disposed when child A2 is pushed.
168-172
: LGTM!The test correctly verifies that child B is disposed when it is popped.
179-186
: LGTM!The test correctly verifies that child A2 is disposed when it is popped.
decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenFactory.kt (1)
94-95
: Documentation Update: Restored Navigation State RequirementsThe comment clarifies the requirements for the restored navigation state, ensuring that it must have the same amount of child configurations in the same order.
decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenBasicTest.kt (6)
222-230
: Test Added: Verify Duplicated Children AdditionThe test correctly sets the
duplicateConfigurationsEnabled
flag and verifies that duplicated children are added.
232-254
: Test Added: Verify Duplicated Children Removal from EndThe test correctly sets the
duplicateConfigurationsEnabled
flag and verifies that duplicated children are removed from the end.
256-277
: Test Added: Verify Duplicated Instances Removal from EndThe test correctly sets the
duplicateConfigurationsEnabled
flag and verifies that duplicated instances are removed from the end.
279-296
: Test Added: Verify Duplicated Children Removal from StartThe test correctly sets the
duplicateConfigurationsEnabled
flag and verifies that duplicated children are removed from the start.
297-316
: Test Added: Verify Duplicated Instances Removal from StartThe test correctly sets the
duplicateConfigurationsEnabled
flag and verifies that duplicated instances are removed from the start.
286-294
: Test Added: Verify Last Duplicated Child DestructionThe test correctly sets the
duplicateConfigurationsEnabled
flag and verifies that the last duplicated child is destroyed when removed.decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenLifecycleTest.kt (4)
286-294
: Test Added: Verify Last Duplicated Child DestructionThe test correctly sets the
duplicateConfigurationsEnabled
flag and verifies that the last duplicated child is destroyed when removed.
296-305
: Test Added: Verify First Duplicated Child CreationThe test correctly sets the
duplicateConfigurationsEnabled
flag and verifies that the first duplicated child is created when the last duplicated child is removed.
307-316
: Test Added: Verify Last Duplicated Child DestructionThe test correctly sets the
duplicateConfigurationsEnabled
flag and verifies that the last duplicated child is destroyed when the first duplicated child is removed.
318-327
: Test Added: Verify First Duplicated Child ResumptionThe test correctly sets the
duplicateConfigurationsEnabled
flag and verifies that the first duplicated child is resumed when the first duplicated child is removed.decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenNavigator.kt (10)
36-40
: Feature Update: Experimental Flag for Duplicate ConfigurationsThe function correctly uses the
duplicateConfigurationsEnabled
flag to determine the behavior for duplicate configurations.
42-50
: Function Added: getChildrenDefaultThe function correctly maps the items to children using the default behavior.
52-60
: Function Added: getChildrenExperimentalThe function correctly maps the items to children using the experimental behavior for duplicate configurations.
88-93
: Feature Update: Experimental Flag for Restoring Duplicate ConfigurationsThe function correctly uses the
duplicateConfigurationsEnabled
flag to determine the behavior for restoring duplicate configurations.
Line range hint
95-118
: Function Added: restoreDefaultThe function correctly restores the default behavior for child items.
119-146
: Function Added: restoreExperimentalThe function correctly restores the experimental behavior for child items with duplicate configurations.
188-193
: Feature Update: Experimental Flag for Switching Duplicate ConfigurationsThe function correctly uses the
duplicateConfigurationsEnabled
flag to determine the behavior for switching duplicate configurations.
195-206
: Function Added: switchDefaultThe function correctly switches the default behavior for child items.
Line range hint
207-250
: Function Added: prepareNewItemsDefaultThe function correctly prepares new items using the default behavior.
Line range hint
251-262
: Function Added: destroyOldItemsDefaultThe function correctly destroys old items using the default behavior.
decompose/src/webTest/kotlin/com/arkivanov/decompose/router/stack/webhistory/DefaultWebHistoryControllerTest.kt (5)
67-80
: Test case looks good.The test case correctly verifies that pushing the same configuration results in the URL being pushed to history.
97-110
: Test case looks good.The test case correctly verifies that popping from a router with two identical configurations changes the history to the previous page.
129-144
: Test case looks good.The test case correctly verifies that popping after pushing the same configuration changes the history to the previous page.
97-110
: Test case looks good.The test case correctly verifies that popping from a router with two identical configurations changes the history to the previous page.
Line range hint
421-430
: Test case looks good.The test case correctly verifies that pushing duplicated children results in a stack containing duplicated children.
decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/stack/ChildStackIntegrationTest.kt (2)
421-430
: Test case looks good.The test case correctly verifies that pushing duplicated children results in a stack containing duplicated children.
432-441
: Test case looks good.The test case correctly verifies that removing duplicated children results in a stack containing unique children.
decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenSavedStateTest.kt (2)
556-575
: Test case looks good.The test case correctly verifies that states are restored correctly when the first and last children are duplicated and the state is recreated.
577-592
: Test case looks good.The test case correctly verifies that states are restored correctly when the first and last children are duplicated and the children are recreated.
decompose/api/jvm/decompose.api (6)
32-32
: Verify usage ofcomponent2-LXPJe6Q
method inChild$Destroyed
classEnsure that all instances of
Child$Destroyed
correctly utilize thecomponent2-LXPJe6Q
method.
40-40
: Verify usage ofgetKey-LXPJe6Q
method inChild$Destroyed
classEnsure that all instances of
Child$Destroyed
correctly utilize thegetKey-LXPJe6Q
method.
34-34
: Verify usage ofcopy-21uEA-E
method inChild$Destroyed
classEnsure that all instances of
Child$Destroyed
correctly utilize thecopy-21uEA-E
method.
23-23
: Verify usage ofgetKey-LXPJe6Q
method inChild$Created
classEnsure that all instances of
Child$Created
correctly utilize thegetKey-LXPJe6Q
method.
18-18
: Verify usage ofcopy-qAgt5DY
method inChild$Created
classEnsure that all instances of
Child$Created
correctly utilize thecopy-qAgt5DY
method.Verification successful
Method
copy-qAgt5DY
corresponds to thecopy
method inChild$Created
classThe
copy-qAgt5DY
method mentioned in the review comment corresponds to thecopy
method in theChild$Created
class, which is annotated with@Deprecated
for binary compatibility. The method is defined and used correctly.
- File:
decompose/src/commonMain/kotlin/com/arkivanov/decompose/Child.kt
- Class:
Child.Created
- Method:
copy
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of the `copy-qAgt5DY` method in `Child$Created` class. # Test: Search for the method usage. Expect: All usages of `Child$Created` should include `copy-qAgt5DY`. rg --type kotlin $'copy-qAgt5DY'Length of output: 31
Script:
#!/bin/bash # Description: Search for the `Child$Created` class and list its methods. # Find the `Child$Created` class definition and list all its methods. ast-grep --lang kotlin --pattern $'class Child$Created { $$$ }' --json | jq -r '.[].matches[].text' | grep -oP 'fun \K\w+(?=\()'Length of output: 131
Script:
#!/bin/bash # Description: Search for all class definitions and method declarations in the Kotlin codebase. # Find all class definitions in Kotlin files. ast-grep --lang kotlin --pattern $'class $_ { $$$ }' --json | jq -r '.[].matches[].text' # Find all method declarations in Kotlin files. ast-grep --lang kotlin --pattern $'fun $_($_) { $$$ }' --json | jq -r '.[].matches[].text'Length of output: 305
Script:
#!/bin/bash # Description: List all Kotlin files in the repository. # List all Kotlin files in the repository. fd -e ktLength of output: 32991
Script:
#!/bin/bash # Description: Search for the `Child$Created` class and its methods in the specific Kotlin file. # Find the `Child$Created` class definition and list all its methods in the specified file. ast-grep --lang kotlin --pattern $'class Child$Created { $$$ }' decompose/src/commonMain/kotlin/com/arkivanov/decompose/Child.kt --json | jq -r '.[].matches[].text' | grep -oP 'fun \K\w+(?=\()'Length of output: 196
Script:
#!/bin/bash # Description: Display the contents of the `decompose/src/commonMain/kotlin/com/arkivanov/decompose/Child.kt` file for manual inspection. # Display the contents of the specified file. cat decompose/src/commonMain/kotlin/com/arkivanov/decompose/Child.ktLength of output: 1630
16-16
: Verify usage ofcomponent3-LXPJe6Q
method inChild$Created
classEnsure that all instances of
Child$Created
correctly utilize thecomponent3-LXPJe6Q
method.decompose/api/android/decompose.api (6)
8-8
: Verify the naming convention and purpose of the new method.The method
getKey-LXPJe6Q
follows a different naming convention compared to existing methods. Ensure this is intentional and aligns with the new key management system.
13-13
: Verify the necessity and usage of the synthetic constructor.The synthetic constructor added to
Child$Created
should be verified for its necessity and usage within the codebase.
16-16
: Verify the naming convention and purpose of the new method.The method
component3-LXPJe6Q
follows a different naming convention compared to existing methods. Ensure this is intentional and aligns with the new key management system.
23-23
: Verify the naming convention and purpose of the new method.The method
getKey-LXPJe6Q
follows a different naming convention compared to existing methods. Ensure this is intentional and aligns with the new key management system.
30-30
: Verify the necessity and usage of the synthetic constructor.The synthetic constructor added to
Child$Destroyed
should be verified for its necessity and usage within the codebase.
32-32
: Verify the naming convention and purpose of the new method.The method
component2-LXPJe6Q
follows a different naming convention compared to existing methods. Ensure this is intentional and aligns with the new key management system.decompose/api/decompose.klib.api (4)
129-129
: LGTM! Ensure proper usage of the new constructor.The new constructor for
ChildStack
withChildKey
parameter is approved.However, ensure that all instances where
ChildStack
is instantiated, theChildKey
parameter is correctly utilized.
237-241
: LGTM! Ensure proper usage of the experimental flag.The
DecomposeExperimentFlags
object with theduplicateConfigurationsEnabled
property is approved.However, ensure that the flag is correctly utilized to manage experimental features throughout the codebase.
250-257
: LGTM! Ensure proper usage of theChildKey
class.The
ChildKey
value class with its methods is approved.However, ensure that the
ChildKey
class is correctly utilized to uniquely identify child entities throughout the codebase.
275-311
: LGTM! Ensure proper usage of thekey
property.The new
key
property inChild
and its subclasses (Created
andDestroyed
) is approved.However, ensure that the
key
property is correctly utilized to uniquely identify child entities throughout the codebase.
.../kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/AbstractStackAnimation.kt
Outdated
Show resolved
Hide resolved
...ions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/Pages.kt
Show resolved
Hide resolved
...ns-android/src/main/java/com/arkivanov/decompose/extensions/android/stack/StackRouterView.kt
Show resolved
Hide resolved
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.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (25)
- decompose/api/android/decompose.api (3 hunks)
- decompose/api/decompose.klib.api (4 hunks)
- decompose/api/jvm/decompose.api (3 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/Child.kt (1 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/ChildKey.kt (1 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/DecomposeExperimentFlags.kt (1 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/GettingList.kt (1 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenFactory.kt (1 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenNavigator.kt (7 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/stack/ChildStack.kt (2 hunks)
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenBasicTest.kt (3 hunks)
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenLifecycleTest.kt (2 hunks)
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenRetainedInstanceTest.kt (18 hunks)
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenSavedStateTest.kt (2 hunks)
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenTestBase.kt (4 hunks)
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/stack/ChildStackIntegrationTest.kt (18 hunks)
- decompose/src/webTest/kotlin/com/arkivanov/decompose/router/stack/webhistory/DefaultWebHistoryControllerTest.kt (3 hunks)
- extensions-android/src/main/java/com/arkivanov/decompose/extensions/android/stack/StackRouterView.kt (5 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/Pages.kt (3 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/Children.kt (4 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/AbstractStackAnimation.kt (4 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/MovableStackAnimation.kt (1 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/SimpleStackAnimation.kt (1 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/predictiveback/PredictiveBackAnimation.kt (2 hunks)
- extensions-compose/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/stack/ChildrenTest.kt (5 hunks)
Files skipped from review as they are similar to previous changes (21)
- decompose/api/decompose.klib.api
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/ChildKey.kt
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/DecomposeExperimentFlags.kt
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/GettingList.kt
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenFactory.kt
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/stack/ChildStack.kt
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenBasicTest.kt
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenLifecycleTest.kt
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenRetainedInstanceTest.kt
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenSavedStateTest.kt
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenTestBase.kt
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/stack/ChildStackIntegrationTest.kt
- decompose/src/webTest/kotlin/com/arkivanov/decompose/router/stack/webhistory/DefaultWebHistoryControllerTest.kt
- extensions-android/src/main/java/com/arkivanov/decompose/extensions/android/stack/StackRouterView.kt
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/Pages.kt
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/Children.kt
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/AbstractStackAnimation.kt
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/MovableStackAnimation.kt
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/SimpleStackAnimation.kt
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/predictiveback/PredictiveBackAnimation.kt
- extensions-compose/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/stack/ChildrenTest.kt
Additional comments not posted (25)
decompose/src/commonMain/kotlin/com/arkivanov/decompose/Child.kt (8)
8-9
: LGTM! The use of@ExperimentalDecomposeApi
annotation is appropriate.The
key
property is marked as experimental, indicating that it is a new and possibly unstable feature.
11-11
: LGTM! The use of@ExperimentalDecomposeApi
annotation is appropriate.The constructor of the
Created
class is marked as experimental, indicating that it is a new and possibly unstable feature.
15-17
: LGTM! The use of@property:ExperimentalDecomposeApi
annotation is appropriate.The
key
property in theCreated
class is marked as experimental, indicating that it is a new and possibly unstable feature.
18-22
: LGTM! The secondary constructor is a useful addition.The constructor allows creating a
Created
instance with aChildKey
derived from the configuration, enhancing convenience and consistency.
24-27
: LGTM! The deprecatedcopy
method ensures binary compatibility.This method maintains backward compatibility while guiding users towards the new method.
29-33
: LGTM! The use of@ExperimentalDecomposeApi
annotation is appropriate.The constructor and the
key
property in theDestroyed
class are marked as experimental, indicating that they are new and possibly unstable features.
35-38
: LGTM! The secondary constructor is a useful addition.The constructor allows creating a
Destroyed
instance with aChildKey
derived from the configuration, enhancing convenience and consistency.
42-44
: LGTM! The deprecatedcopy
method ensures binary compatibility.This method maintains backward compatibility while guiding users towards the new method.
decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenNavigator.kt (10)
4-6
: LGTM! The new imports are necessary and correctly added.The imports for
ChildKey
,DecomposeExperimentFlags
, andkeyed
are required for the new functionality introduced in the file.
36-40
: LGTM! The conditional check for the experimental flag is correctly implemented.The code correctly determines whether to call
getChildrenExperimental
orgetChildrenDefault
based on theduplicateConfigurationsEnabled
flag.
42-51
: LGTM! ThegetChildrenDefault
method is correctly implemented.The method correctly maps items to
Child.Created
orChild.Destroyed
based on the instance, maintaining the existing behavior when the experimental flag is disabled.
52-60
: LGTM! ThegetChildrenExperimental
method is correctly implemented.The method correctly maps items to
Child
objects using keyed configurations, ensuring that the experimental feature is implemented as intended.
88-93
: LGTM! The conditional check for the experimental flag is correctly implemented.The code correctly determines whether to call
restoreExperimental
orrestoreDefault
based on theduplicateConfigurationsEnabled
flag.
119-146
: LGTM! TherestoreExperimental
method is correctly implemented.The method correctly handles the restoration of children using keyed configurations, ensuring that the experimental feature is implemented as intended.
188-193
: LGTM! The conditional check for the experimental flag is correctly implemented.The code correctly determines whether to call
switchExperimental
orswitchDefault
based on theduplicateConfigurationsEnabled
flag.
263-269
: LGTM! TheswitchExperimental
method is correctly implemented.The method correctly handles switching the state of children using keyed configurations, ensuring that the experimental feature is implemented as intended.
271-313
: LGTM! TheprepareNewItemsExperimental
method is correctly implemented.The method correctly handles the preparation of new items using keyed configurations, ensuring that the experimental feature is implemented as intended.
315-325
: LGTM! ThedestroyOldItemsExperimental
method is correctly implemented.The method correctly handles the destruction of old items using keyed configurations, ensuring that the experimental feature is implemented as intended.
decompose/api/jvm/decompose.api (1)
8-8
: Verify the implementation of thegetKey-LXPJe6Q
method in all classes that implement theChild
interface.The addition of this method to the
Child
interface is necessary for the new functionality involvingChildKey
. However, ensure that all classes implementingChild
provide a meaningful implementation of this method.decompose/api/android/decompose.api (6)
30-30
: New Constructor Added: Ensure Binary CompatibilityA new synthetic constructor has been added to
Child$Destroyed
. Ensure that this change maintains binary compatibility with any existing bytecode that relies on the previous constructor.
13-13
: New Constructor Added: Ensure Binary CompatibilityA new synthetic constructor has been added to
Child$Created
. Ensure that this change maintains binary compatibility with any existing bytecode that relies on the previous constructor.
16-19
: New Methods Added: Ensure Correct FunctionalityNew methods
component3-LXPJe6Q
,copy
,copy-qAgt5DY
, andcopy-qAgt5DY$default
have been added toChild$Created
. Verify that these methods function correctly and are used consistently across the codebase.
45-57
: New Class Added: Ensure Correct Implementation and UsageThe
ChildKey
class has been added with methods for key management. Verify that these methods are correctly implemented and used consistently across the codebase.
75-79
: New Class Added: Ensure Correct Implementation and UsageThe
DecomposeExperimentFlags
class has been added with methods to manage experimental features. Verify that these methods are correctly implemented and used consistently across the codebase.
32-35
: New Methods Added: Ensure Correct FunctionalityNew methods
component2-LXPJe6Q
,copy
,copy-21uEA-E
, andcopy-21uEA-E$default
have been added toChild$Destroyed
. Verify that these methods function correctly and are used consistently across the codebase.
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.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (26)
- decompose/api/android/decompose.api (3 hunks)
- decompose/api/decompose.klib.api (3 hunks)
- decompose/api/jvm/decompose.api (3 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/Child.kt (1 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/DecomposeExperimentFlags.kt (1 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/GettingList.kt (1 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/Utils.kt (1 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenFactory.kt (1 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenNavigator.kt (7 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/stack/ChildStack.kt (2 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/stack/StackNavigatorExt.kt (2 hunks)
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenBasicTest.kt (3 hunks)
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenLifecycleTest.kt (2 hunks)
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenRetainedInstanceTest.kt (18 hunks)
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenSavedStateTest.kt (2 hunks)
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenTestBase.kt (4 hunks)
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/stack/ChildStackIntegrationTest.kt (18 hunks)
- decompose/src/webTest/kotlin/com/arkivanov/decompose/router/stack/webhistory/DefaultWebHistoryControllerTest.kt (3 hunks)
- extensions-android/src/main/java/com/arkivanov/decompose/extensions/android/stack/StackRouterView.kt (4 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/Pages.kt (3 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/Children.kt (4 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/AbstractStackAnimation.kt (4 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/MovableStackAnimation.kt (1 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/SimpleStackAnimation.kt (1 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/predictiveback/PredictiveBackAnimation.kt (1 hunks)
- extensions-compose/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/stack/ChildrenTest.kt (4 hunks)
Files skipped from review as they are similar to previous changes (23)
- decompose/api/android/decompose.api
- decompose/api/decompose.klib.api
- decompose/api/jvm/decompose.api
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/Child.kt
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/DecomposeExperimentFlags.kt
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/GettingList.kt
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenFactory.kt
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/stack/ChildStack.kt
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenBasicTest.kt
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenLifecycleTest.kt
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenRetainedInstanceTest.kt
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenSavedStateTest.kt
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/children/ChildrenTestBase.kt
- decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/stack/ChildStackIntegrationTest.kt
- decompose/src/webTest/kotlin/com/arkivanov/decompose/router/stack/webhistory/DefaultWebHistoryControllerTest.kt
- extensions-android/src/main/java/com/arkivanov/decompose/extensions/android/stack/StackRouterView.kt
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/Pages.kt
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/Children.kt
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/AbstractStackAnimation.kt
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/MovableStackAnimation.kt
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/SimpleStackAnimation.kt
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/predictiveback/PredictiveBackAnimation.kt
- extensions-compose/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/stack/ChildrenTest.kt
Additional comments not posted (11)
decompose/src/commonMain/kotlin/com/arkivanov/decompose/Utils.kt (2)
14-23
: Verify the use ofPair
as a map key.Using a
Pair
as a key in the map might have implications on the performance and correctness. Ensure that this approach is suitable for your use case.
14-23
: LGTM!The code changes are approved.
decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/stack/StackNavigatorExt.kt (2)
14-16
: LGTM!The comments provide clear information about the experimental feature for duplicate configurations.
29-31
: LGTM!The comments provide clear information about the experimental feature for duplicate configurations.
decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/children/ChildrenNavigator.kt (7)
35-39
: LGTM! But verify the usage of the experimental flag.The code changes are approved.
However, ensure that the experimental flag
duplicateConfigurationsEnabled
is correctly used and tested.
41-50
: LGTM!The code changes are approved.
51-59
: LGTM!The code changes are approved.
87-91
: LGTM! But verify the usage of the experimental flag.The code changes are approved.
However, ensure that the experimental flag
duplicateConfigurationsEnabled
is correctly used and tested.
118-145
: LGTM!The code changes are approved.
187-191
: LGTM! But verify the usage of the experimental flag.The code changes are approved.
However, ensure that the experimental flag
duplicateConfigurationsEnabled
is correctly used and tested.
262-268
: LGTM!The code changes are approved.
Summary by CodeRabbit
New Features
DecomposeExperimentFlags
class to manage experimental features.ChildKey
class to handle child components with unique keys.Bug Fixes
Chores