Predictive back #441
Predictive back #441
Conversation
WalkthroughThe changes enable predictive back navigation by setting Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SystemUI
participant MainActivity
participant Fragment
participant SeekableTransition
User->>SystemUI: Initiate back gesture
SystemUI->>MainActivity: onBackInvoked callback (predictive)
MainActivity->>Fragment: Handle back navigation
Fragment->>SeekableTransition: Animate transition (seekable)
SeekableTransition-->>Fragment: Update view scale/alpha per gesture progress
SystemUI->>User: Show predictive back animation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
app/src/main/java/app/simple/inure/decorations/transitions/SeekableSharedAxisZTransition.java (2)
34-36: Consider narrowing the ImageView exclusion scope.Returning
nullfor allImageViewinstances might cause inconsistent visual behavior where some views animate while images don't. Consider either:
- Allowing ImageView animations but with different parameters
- Adding a more specific condition (e.g., checking for specific tags or IDs)
- Documenting why ImageViews are excluded
15-18: Consider making animation parameters configurable.The scale values and duration are hardcoded, which limits customization. Consider adding setter methods or constructor parameters for these values to make the transition more flexible.
app/src/main/java/app/simple/inure/activities/app/MainActivity.kt (1)
6-6: Remove unused import.The import
android.net.wifi.hotspot2.pps.HomeSpis not used anywhere in the file and should be removed to keep the code clean.-import android.net.wifi.hotspot2.pps.HomeSp
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/src/github/AndroidManifest.xml(1 hunks)app/src/main/AndroidManifest.xml(1 hunks)app/src/main/java/app/simple/inure/activities/app/MainActivity.kt(2 hunks)app/src/main/java/app/simple/inure/decorations/transitions/SeekableSharedAxisZTransition.java(1 hunks)app/src/main/java/app/simple/inure/extensions/fragments/ScopedFragment.kt(5 hunks)
🔇 Additional comments (6)
app/src/github/AndroidManifest.xml (1)
65-65: LGTM! Correctly enables predictive back callback.This change properly enables the
OnBackInvokedCallbackfeature for Android 14+ predictive back gestures, which aligns with the PR objective to support predictive back animations.app/src/main/AndroidManifest.xml (1)
64-64: LGTM! Consistent with github variant.Correctly enables predictive back callbacks for the main build variant, maintaining consistency with the github variant manifest.
app/src/main/java/app/simple/inure/extensions/fragments/ScopedFragment.kt (4)
44-44: LGTM! Correctly imports the new transition class.
297-300: LGTM! Proper integration of seekable transition.The transition type configuration correctly uses the new
SeekableSharedAxisZTransitionwith appropriate direction flags for each transition phase (enter, exit, reenter, return).
626-626: LGTM! Enables visual feedback during back gestures.Uncommenting this line provides important visual feedback during predictive back gestures by updating the view's alpha based on progress.
752-752: Verify intentional use ofadd()instead ofreplace()A global search confirms that every other fragment transaction against
R.id.app_containerusesreplace(). This one-off switch toadd()changes the lifecycle and back-stack semantics—existing fragments aren’t removed, and callbacks likeonPause()/onStop()won’t fire on the hidden instance.Please confirm this is deliberate and that you’ve tested:
- Memory usage when stacking fragments rather than replacing
- Proper invocation of lifecycle callbacks on both old and new fragments
- Back-navigation behavior and UI layering (especially for arc transitions)
Also consider adding a brief code comment or updating documentation to explain why
add()is required here.
| if (AppUtils.isDebug()) { | ||
| supportFragmentManager.beginTransaction() | ||
| .replace(R.id.app_container, SplashScreen.newInstance(false), SplashScreen.TAG) | ||
| .replace(R.id.app_container, Home.newInstance(), SplashScreen.TAG) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify fragment tag consistency with the new fragment type.
The code now instantiates Home.newInstance() but still uses SplashScreen.TAG as the fragment tag. This creates a mismatch between the actual fragment type and its tag, which could lead to confusion when debugging or managing fragment transactions.
Consider either:
- Using
Home.TAGinstead ofSplashScreen.TAGfor consistency - Or keeping
SplashScreen.TAGif there's a specific reason (e.g., maintaining backward compatibility with existing fragment management logic)
Also applies to: 250-250
🏁 Script executed:
#!/bin/bash
# Search for usage of SplashScreen.TAG to understand if this tag is used elsewhere
# and whether changing it would break existing functionality
rg -A 3 -B 3 "SplashScreen\.TAG"
# Also check if Home.TAG exists and how it's used
rg -A 3 -B 3 "Home\.TAG"Length of output: 2252
Align Home fragment tag with its type in MainActivity
The debug and non-debug branches for isNewIntent are still using SplashScreen.TAG when replacing with Home.newInstance(). Update both occurrences to use Home.TAG for clarity and consistency.
• File: app/src/main/java/app/simple/inure/activities/app/MainActivity.kt
– Around lines 246 and 250
Diff:
- .replace(R.id.app_container, Home.newInstance(), SplashScreen.TAG)
+ .replace(R.id.app_container, Home.newInstance(), Home.TAG)If there’s a deliberate reason to continue using SplashScreen.TAG here (e.g., legacy logic), please document it in a comment.
🤖 Prompt for AI Agents
In app/src/main/java/app/simple/inure/activities/app/MainActivity.kt around
lines 246 and 250, the fragment replacement calls for Home.newInstance()
incorrectly use SplashScreen.TAG as the tag. Update both occurrences to use
Home.TAG instead to maintain clarity and consistency with the fragment type. If
there is a specific reason to keep using SplashScreen.TAG, add a comment
explaining this decision.
| private Animator createAnimator(final View view, final float startScale, final float endScale, | ||
| final float startAlpha, final float endAlpha) { | ||
| if (view instanceof ImageView) { | ||
| return null; | ||
| } | ||
|
|
||
| ValueAnimator animator = ValueAnimator.ofFloat(0f, 1f); | ||
| animator.setDuration(DEFAULT_DURATION); | ||
| animator.setInterpolator(new DecelerateInterpolator(DECELERATE_FACTOR)); | ||
| animator.addUpdateListener(animation -> { | ||
| float progress = (float) animation.getAnimatedValue(); | ||
| float scale = startScale + (endScale - startScale) * progress; | ||
| float alpha = startAlpha + (endAlpha - startAlpha) * progress; | ||
| view.setScaleX(scale); | ||
| view.setScaleY(scale); | ||
| view.setAlpha(alpha); | ||
| }); | ||
|
|
||
| return animator; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null safety check for the view parameter.
The createAnimator method should validate the view parameter to prevent potential NPEs.
private Animator createAnimator(final View view, final float startScale, final float endScale,
final float startAlpha, final float endAlpha) {
+ if (view == null) {
+ return null;
+ }
if (view instanceof ImageView) {
return null;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private Animator createAnimator(final View view, final float startScale, final float endScale, | |
| final float startAlpha, final float endAlpha) { | |
| if (view instanceof ImageView) { | |
| return null; | |
| } | |
| ValueAnimator animator = ValueAnimator.ofFloat(0f, 1f); | |
| animator.setDuration(DEFAULT_DURATION); | |
| animator.setInterpolator(new DecelerateInterpolator(DECELERATE_FACTOR)); | |
| animator.addUpdateListener(animation -> { | |
| float progress = (float) animation.getAnimatedValue(); | |
| float scale = startScale + (endScale - startScale) * progress; | |
| float alpha = startAlpha + (endAlpha - startAlpha) * progress; | |
| view.setScaleX(scale); | |
| view.setScaleY(scale); | |
| view.setAlpha(alpha); | |
| }); | |
| return animator; | |
| } | |
| private Animator createAnimator(final View view, final float startScale, final float endScale, | |
| final float startAlpha, final float endAlpha) { | |
| if (view == null) { | |
| return null; | |
| } | |
| if (view instanceof ImageView) { | |
| return null; | |
| } | |
| ValueAnimator animator = ValueAnimator.ofFloat(0f, 1f); | |
| animator.setDuration(DEFAULT_DURATION); | |
| animator.setInterpolator(new DecelerateInterpolator(DECELERATE_FACTOR)); | |
| animator.addUpdateListener(animation -> { | |
| float progress = (float) animation.getAnimatedValue(); | |
| float scale = startScale + (endScale - startScale) * progress; | |
| float alpha = startAlpha + (endAlpha - startAlpha) * progress; | |
| view.setScaleX(scale); | |
| view.setScaleY(scale); | |
| view.setAlpha(alpha); | |
| }); | |
| return animator; | |
| } |
🤖 Prompt for AI Agents
In
app/src/main/java/app/simple/inure/decorations/transitions/SeekableSharedAxisZTransition.java
around lines 32 to 51, the createAnimator method lacks a null check for the view
parameter, which can cause NullPointerExceptions. Add a null check at the start
of the method to return null or handle the case appropriately if view is null
before proceeding with any operations on it.
closes #329
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores