-
-
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
Don't restart Activity stack when there is no deep link #854
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThe changes update the Changes
Sequence Diagram(s)sequenceDiagram
participant I as Intent
participant D as DeeplinkUtils
participant R as RestartChecker
I->>D: Pass intent
D->>D: Extract intentData from intent
alt intentData exists
D->>R: Check restartIfNeeded()
alt Restart needed
R-->>D: Returns true
D->>D: Assign deepLink from intentData and process deep link
else Restart not needed
D->>D: Skip deep link processing
end
else No intentData
D->>D: Skip deep link processing
end
Poem
✨ Finishing Touches
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 using PR comments)
Other keywords and placeholders
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
🧹 Nitpick comments (1)
decompose/src/androidMain/kotlin/com/arkivanov/decompose/DeeplinkUtils.kt (1)
11-46
: Consider updating the function documentationWhile the implementation changes are good, the function documentation hasn't been updated to reflect the new behavior. Consider updating the documentation to mention that the Activity stack restart only happens when there's a deep link.
* It is strongly recommended to always use the `standard` (default) * [launchMode](https://developer.android.com/guide/components/activities/tasks-and-back-stack#TaskLaunchModes) * for the [Activity] when handling deep links. This function takes care of -* restarting the [Activity] task and finishing this [Activity] if needed, +* restarting the [Activity] task and finishing this [Activity] if needed (only when a deep link is present), * in which case the returned value is `null`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
decompose/src/androidMain/kotlin/com/arkivanov/decompose/DeeplinkUtils.kt
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build on Linux
- GitHub Check: Build on macOS
🔇 Additional comments (2)
decompose/src/androidMain/kotlin/com/arkivanov/decompose/DeeplinkUtils.kt (2)
51-53
: Improvement to prevent unnecessary Activity stack restartsThis change ensures that the Activity stack is only restarted when there's actual deep link data available, which is a good optimization.
By storing the intent data in a local variable and only restarting when both
intentData != null
andrestartIfNeeded()
is true, unnecessary restarts are avoided when no deep link exists.
59-59
: Consistent use of the local variableThe code now properly uses the stored
intentData
variable rather than accessingintent.data
again, which is more consistent and slightly more efficient.
Summary by CodeRabbit