-
-
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
Renamed Pages composable function to ChildPages, promoted all Child Pages API to stable #756
Conversation
WalkthroughThe recent changes enhance the stability of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChildPages
participant Navigator
User->>ChildPages: Request to display child pages
ChildPages->>Navigator: Route to selected child page
Navigator->>ChildPages: Render the requested child page
ChildPages-->>User: Display child page content
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 Configuration 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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (13)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/ChildPages.kt (2 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/ChildPagesFactory.kt (4 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/Pages.kt (1 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/PagesNavigation.kt (2 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/PagesNavigator.kt (1 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/PagesNavigatorExt.kt (7 hunks)
- extensions-compose/api/android/extensions-compose.api (1 hunks)
- extensions-compose/api/extensions-compose.klib.api (1 hunks)
- extensions-compose/api/jvm/extensions-compose.api (1 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/ChildPages.kt (8 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/PagesScrollAnimation.kt (1 hunks)
- extensions-compose/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/pages/ChildPagesTest.kt (3 hunks)
- sample/shared/compose/src/commonMain/kotlin/com/arkivanov/sample/shared/pages/PagesContent.kt (2 hunks)
Files skipped from review due to trivial changes (7)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/ChildPages.kt
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/Pages.kt
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/PagesNavigation.kt
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/PagesNavigator.kt
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/PagesNavigatorExt.kt
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/PagesScrollAnimation.kt
- sample/shared/compose/src/commonMain/kotlin/com/arkivanov/sample/shared/pages/PagesContent.kt
Additional comments not posted (16)
extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/ChildPages.kt (3)
Line range hint
141-145
:
LGTM!The removal of the experimental annotation indicates the API is now stable.
Line range hint
25-36
:
LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
ChildPages
match the new signature.
Line range hint
51-95
:
LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
ChildPages
match the new signature.extensions-compose/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/pages/ChildPagesTest.kt (2)
Line range hint
150-156
:
LGTM!The test function
setContent
is correctly updated to useChildPages
.
17-17
: LGTM! But verify the function usage in the codebase.The class and its test functions are correctly renamed.
However, ensure that all function calls to
ChildPages
match the new signature.Verification successful
Function calls to
ChildPages
match the new signature.The instances of
ChildPages
across the codebase have been reviewed and they all use the correct signature.
- Verified in:
sample/shared/shared/src/commonMain/kotlin/com/arkivanov/sample/shared/pages/PagesComponent.kt
sample/shared/shared/src/commonMain/kotlin/com/arkivanov/sample/shared/pages/DefaultPagesComponent.kt
extensions-compose/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/pages/ChildPagesTest.kt
extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/ChildPages.kt
decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/pages/ChildPagesBackButtonTest.kt
decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/pages/ChildPagesSelectLastTest.kt
decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/pages/ChildPagesSelectFirstTest.kt
decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/pages/ChildPagesSelectTest.kt
decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/pages/ChildPagesSelectPrevTest.kt
decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/pages/ChildPagesSelectNextTest.kt
decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/pages/ChildPagesSavedStateTest.kt
decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/pages/ChildPagesClearTest.kt
decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/pages/BaseChildPagesTest.kt
decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/pages/ChildPagesIntegrationTest.kt
decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/ChildPagesFactory.kt
decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/ChildPages.kt
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `ChildPages` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type kotlin -A 5 $'ChildPages'Length of output: 33910
decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/ChildPagesFactory.kt (3)
Line range hint
26-51
:
LGTM!The removal of the experimental annotation indicates the API is now stable.
Line range hint
72-114
:
LGTM!The removal of the experimental annotation indicates the API is now stable.
Line range hint
116-116
:
LGTM! But verify the class usage in the codebase.The removal of the class
PagesSavedNavState
suggests a simplification or refactoring of state management.However, ensure that the removal does not affect other parts of the codebase.
Verification successful
LGTM! The removal of the
PagesSavedNavState
class does not affect other parts of the codebase.
- The search command confirmed that there are no occurrences of
PagesSavedNavState
in the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of `PagesSavedNavState` in the codebase. # Test: Search for the class usage. Expect: No occurrences of the class. rg --type kotlin -A 5 $'PagesSavedNavState'Length of output: 42
extensions-compose/api/android/extensions-compose.api (3)
8-9
: LGTM! Verify the usage ofdefaultHorizontalPager
anddefaultVerticalPager
.The code changes are approved.
However, ensure that the new functions
defaultHorizontalPager
anddefaultVerticalPager
are used correctly in the codebase.Verification successful
Verified the usage of
defaultHorizontalPager
anddefaultVerticalPager
.The functions are correctly defined and used within the
ChildPages.kt
file.
defaultHorizontalPager
is used twice within theChildPages
composable function.defaultVerticalPager
is defined and used correctly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `defaultHorizontalPager` and `defaultVerticalPager` functions in the codebase. # Test: Search for the function usage. Expect: Correct usage of the new functions. rg --type kotlin -A 5 $'defaultHorizontalPager' rg --type kotlin -A 5 $'defaultVerticalPager'Length of output: 3450
Script:
#!/bin/bash # Extract more context around the usages of `defaultHorizontalPager` and `defaultVerticalPager` in `ChildPages.kt` rg --type kotlin -A 10 -B 10 $'defaultHorizontalPager' extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/ChildPages.kt rg --type kotlin -A 10 -B 10 $'defaultVerticalPager' extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/ChildPages.kt # Search for any other references to `defaultHorizontalPager` across the entire codebase rg --type kotlin $'defaultHorizontalPager' # Search for any other references to `defaultVerticalPager` across the entire codebase rg --type kotlin $'defaultVerticalPager'Length of output: 3301
6-7
: Ensure proper usage of overloadedChildPages
functions.The new
ChildPages
functions provide enhanced flexibility. Verify that the correct function overload is used in the codebase.Verification successful
Ensure proper usage of overloaded
ChildPages
functions.The new
ChildPages
functions provide enhanced flexibility. Verify that the correct function overload is used in the codebase.
Ensure proper usage of overloaded
ChildPages
functions.The new
ChildPages
functions provide enhanced flexibility. Based on the gathered context, the correct function overloads appear to be used in the codebase.
- In
PagesComponent.kt
,pages
is of typeValue<ChildPages<*, KittenComponent>>
, which matches the first overload.- In
DefaultPagesComponent.kt
,pages
is also of typeValue<ChildPages<*, KittenComponent>>
, which matches the first overload.- In
ChildPagesTest.kt
, the usages ofChildPages
withitems
andselectedIndex
match the second overload.- In
ChildPages.kt
, both overloads are used correctly based on the provided contexts.No issues found with the usage of
ChildPages
overloads.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of overloaded `ChildPages` functions in the codebase. # Test: Search for `ChildPages` function usage. Expect: Correct overloads are used. rg --type kotlin -A 5 $'ChildPages'Length of output: 33910
Script:
#!/bin/bash # Description: Extract function signatures and usage contexts for `ChildPages` to verify correct overload usage. # Extract function signatures of `ChildPages` echo "Function Signatures:" rg --type kotlin --context 5 'fun <C : Any, T : Any> ChildPages' extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/ChildPages.kt # Extract context around each usage of `ChildPages` echo "Usage Contexts:" rg --type kotlin -A 10 -B 2 'ChildPages' sample/shared/shared/src/commonMain/kotlin/com/arkivanov/sample/shared/pages/PagesComponent.kt rg --type kotlin -A 10 -B 2 'ChildPages' sample/shared/shared/src/commonMain/kotlin/com/arkivanov/sample/shared/pages/DefaultPagesComponent.kt rg --type kotlin -A 10 -B 2 'ChildPages' extensions-compose/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/pages/ChildPagesTest.kt rg --type kotlin -A 10 -B 2 'ChildPages' extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/ChildPages.kt rg --type kotlin -A 10 -B 2 'ChildPages' decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/ChildPagesFactory.ktLength of output: 11036
12-13
: LGTM! Ensure singleton pattern is maintained correctly.The code changes are approved.
However, verify that the singleton pattern for
ChildPages
is maintained correctly in the codebase.extensions-compose/api/jvm/extensions-compose.api (3)
10-11
: Ensure proper usage of overloadedChildPages
functions.The new
ChildPages
functions provide enhanced flexibility. Verify that the correct function overload is used in the codebase.
12-13
: LGTM! Verify the usage ofdefaultHorizontalPager
anddefaultVerticalPager
.The code changes are approved.
16-17
: LGTM! Ensure singleton pattern is maintained correctly.The code changes are approved.
extensions-compose/api/extensions-compose.klib.api (2)
89-90
: Ensure proper usage of overloadedChildPages
functions.The new
ChildPages
functions provide enhanced flexibility. Verify that the correct function overload is used in the codebase.
90-90
: Ensure proper usage ofpredictiveBackAnimation
functions.The new
predictiveBackAnimation
functions provide enhanced flexibility. Verify that the correct function overload is used in the codebase.
…ages API to stable
0c9bb36
to
36c5d88
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (13)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/ChildPages.kt (2 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/ChildPagesFactory.kt (4 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/Pages.kt (1 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/PagesNavigation.kt (2 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/PagesNavigator.kt (1 hunks)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/PagesNavigatorExt.kt (7 hunks)
- extensions-compose/api/android/extensions-compose.api (1 hunks)
- extensions-compose/api/extensions-compose.klib.api (1 hunks)
- extensions-compose/api/jvm/extensions-compose.api (1 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/ChildPages.kt (8 hunks)
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/PagesScrollAnimation.kt (1 hunks)
- extensions-compose/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/pages/ChildPagesTest.kt (3 hunks)
- sample/shared/compose/src/commonMain/kotlin/com/arkivanov/sample/shared/pages/PagesContent.kt (2 hunks)
Files skipped from review as they are similar to previous changes (12)
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/ChildPages.kt
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/ChildPagesFactory.kt
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/Pages.kt
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/PagesNavigation.kt
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/PagesNavigator.kt
- decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/pages/PagesNavigatorExt.kt
- extensions-compose/api/extensions-compose.klib.api
- extensions-compose/api/jvm/extensions-compose.api
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/ChildPages.kt
- extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/PagesScrollAnimation.kt
- extensions-compose/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/pages/ChildPagesTest.kt
- sample/shared/compose/src/commonMain/kotlin/com/arkivanov/sample/shared/pages/PagesContent.kt
Additional comments not posted (3)
extensions-compose/api/android/extensions-compose.api (3)
Line range hint
14-29
: Ensure correct implementation of page scroll animations.The class
PagesScrollAnimation
and its subclassesCustom
,Default
, andDisabled
handle page scroll animations. Ensure that the animations are implemented correctly and efficiently.
5-10
: Ensure consistency in function naming and parameters.The class
ChildPagesKt
introduces new functionsChildPages
,defaultHorizontalPager
, anddefaultVerticalPager
. Ensure that the function parameters and naming conventions are consistent with the rest of the codebase.Verification successful
To ensure consistency in function naming and parameters for the
ChildPages
functions, we need to locate their definitions in the main codebase. This will help us verify their consistency with the rest of the codebase.
To locate the definitions of the
ChildPages
functions in the main codebase, we need to perform a broader search. This will help ensure that we are reviewing the correct implementations.
The broader search results show multiple usages and definitions of
ChildPages
across various files. The main implementation ofChildPages
appears to be inChildPages.kt
. The definitions found in this file are crucial for verifying the consistency of function naming and parameters.Ensure consistency in function naming and parameters.
The definitions of
ChildPages
functions inChildPages.kt
should be reviewed to ensure they are consistent with the rest of the codebase.
- File:
extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/pages/ChildPages.kt
- Functions to review:
fun <C : Any, T : Any> ChildPages(pages: Value<ChildPages<C, T>>, ...)
fun <C : Any, T : Any> ChildPages(pages: ChildPages<C, T>, ...)
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of function parameters and naming conventions. # Test: Search for the function definitions. Expect: Consistent naming and parameters. rg --type kotlin -A 5 $'fun ChildPages' rg --type kotlin -A 5 $'fun defaultHorizontalPager' rg --type kotlin -A 5 $'fun defaultVerticalPager'Length of output: 4040
Script:
#!/bin/bash # Description: Locate the definitions of the ChildPages functions in the main codebase. # Search for the definition of the ChildPages function in the main codebase. rg --type kotlin 'fun ChildPages'Length of output: 559
Script:
#!/bin/bash # Description: Perform a broader search to locate the definitions of the ChildPages functions in the main codebase. # Search for the definition of the ChildPages function in all Kotlin files. rg --type kotlin 'ChildPages'Length of output: 6888
12-13
: Verify the singleton pattern implementation.The class
ComposableSingletons$ChildPagesKt
implements a singleton pattern. Ensure that the singleton pattern is correctly implemented and thread-safe.
Closes #752.
Summary by CodeRabbit
New Features
ChildPages
functionality for enhanced child page routing and animation control.ChildPages
functions for greater flexibility in child page management.Bug Fixes
Refactor
Pages
function toChildPages
for clarity.PagesTest
toChildPagesTest
to reflect the new naming.Documentation