ADFA-3718 | Require scrolling to end before project creation#1321
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughRelease Notes - ADFA-3718: Require scrolling to end before project creation
Risks & Best-practice notes
WalkthroughExtracts scroll-end gating into TemplateScrollGateKeeper, moves template validation/execution into ProjectCreationManager, adds a scrollIndicator ImageView, and updates TemplateDetailsFragment to use these utilities for finish gating and project-creation callbacks. ChangesTemplate Details Fragment: Scroll Gate and Project Creation Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/TemplateDetailsFragment.kt`:
- Around line 63-64: The blinking ValueAnimator created in
startBlinkingIndicator() is never stored or stopped, causing a lifecycle leak;
add a nullable property (e.g., indicatorAnimator) to TemplateDetailsFragment to
hold the animator returned by startBlinkingIndicator(), then in onDestroyView()
call indicatorAnimator?.cancel(), indicatorAnimator?.removeAllListeners(),
indicatorAnimator?.removeAllUpdateListeners() and set indicatorAnimator = null
(also clear any animation on the indicator view if present), and update
startBlinkingIndicator() to assign the created animator to this property so it
can be cleaned up when the view is destroyed.
In
`@app/src/main/java/com/itsaky/androidide/utils/ui/TemplateScrollGateKeeper.kt`:
- Around line 59-63: The detach() method calls
recyclerView.viewTreeObserver.removeOnGlobalLayoutListener(globalLayoutListener)
unconditionally which can throw if the ViewTreeObserver is dead; before removing
the listener, guard with recyclerView.viewTreeObserver.isAlive (or obtain the
observer into a local variable and check its isAlive) and only call
removeOnGlobalLayoutListener(globalLayoutListener) when true, leaving the other
cleanup (removeOnScrollListener, removeOnLayoutChangeListener,
onScrollStateChanged = null) unchanged; update TemplateScrollGateKeeper.detach()
to perform this check around the globalLayoutListener removal.
In `@app/src/main/res/layout/fragment_template_details.xml`:
- Around line 70-79: The ImageView with id "scrollIndicator" is decorative and
should be removed from accessibility focus; update the <ImageView> in
fragment_template_details.xml (id scrollIndicator) to mark it decorative by
adding android:contentDescription="`@null`" and
android:importantForAccessibility="no" (and optionally
android:focusable="false") so screen readers ignore the non-actionable guidance
icon.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ec301568-d340-4d1e-ae81-530f23189dc4
📒 Files selected for processing (4)
app/src/main/java/com/itsaky/androidide/fragments/TemplateDetailsFragment.ktapp/src/main/java/com/itsaky/androidide/utils/ProjectCreationManager.ktapp/src/main/java/com/itsaky/androidide/utils/ui/TemplateScrollGateKeeper.ktapp/src/main/res/layout/fragment_template_details.xml
a6f8cfd to
80302c3
Compare
|
Might want to add some TalkBack for visual impaired users. |
7c4683d to
3713bc3
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/fragments/TemplateDetailsFragment.kt (1)
194-203: 💤 Low valueUse the existing
canFinishvariable instead of recalculating.Line 200 duplicates the expression from line 197. Use
canFinishfor consistency.♻️ Suggested fix
private fun updateFinishEnabledState() { val isCreating = viewModel.creatingProject.value ?: false val hasScrolledToBottom = scrollGateKeeper?.hasReachedEnd ?: false val canFinish = !isCreating && hasScrolledToBottom val stateDesc = if (canFinish) null else getString(string.msg_scroll_to_create_project) - binding.finish.isEnabled = !isCreating && hasScrolledToBottom + binding.finish.isEnabled = canFinish binding.scrollIndicator.isVisible = !hasScrolledToBottom ViewCompat.setStateDescription(binding.finish, stateDesc) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/itsaky/androidide/fragments/TemplateDetailsFragment.kt` around lines 194 - 203, The method updateFinishEnabledState() recalculates the same boolean twice; use the already computed canFinish instead of re-evaluating !isCreating && hasScrolledToBottom. Replace the expression assigned to binding.finish.isEnabled with canFinish, keep binding.scrollIndicator.isVisible = !hasScrolledToBottom, and continue calling ViewCompat.setStateDescription(binding.finish, stateDesc) so the logic uses the single source of truth (canFinish) for the finish button enabled state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/TemplateDetailsFragment.kt`:
- Around line 194-203: The method updateFinishEnabledState() recalculates the
same boolean twice; use the already computed canFinish instead of re-evaluating
!isCreating && hasScrolledToBottom. Replace the expression assigned to
binding.finish.isEnabled with canFinish, keep binding.scrollIndicator.isVisible
= !hasScrolledToBottom, and continue calling
ViewCompat.setStateDescription(binding.finish, stateDesc) so the logic uses the
single source of truth (canFinish) for the finish button enabled state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a9fc8886-4808-45e3-8bb3-cbc866669caf
📒 Files selected for processing (5)
app/src/main/java/com/itsaky/androidide/fragments/TemplateDetailsFragment.ktapp/src/main/java/com/itsaky/androidide/utils/ProjectCreationManager.ktapp/src/main/java/com/itsaky/androidide/utils/ui/TemplateScrollGateKeeper.ktapp/src/main/res/layout/fragment_template_details.xmlresources/src/main/res/values/strings.xml
✅ Files skipped from review due to trivial changes (1)
- resources/src/main/res/values/strings.xml
Adds scroll detection to ensure users reach the end of the project setup before continuing.
…the `blinkAnimator` object and accessibility
d53c44d to
2fbf11f
Compare
Description
Implemented a scroll-tracking mechanism to ensure users view all form fields and checkboxes before they can create a project. The "Finish" button is now disabled by default and only enables once the user has scrolled to the bottom of the template details. Additionally, a blinking scroll indicator was added to guide users, and the project creation logic was refactored into a dedicated manager for better maintainability.
Details
TemplateScrollGateKeeperto monitor theRecyclerViewscroll state and detect when the bottom is reached.TemplateDetailsFragmentto bind the gatekeeper to the widgets list and update the "Finish" button's enabled state dynamically.scrollIndicator) next to the "Finish" button that disappears once the user has scrolled to the bottom.ProjectCreationManagerclass.document_4929672108394415729.mp4
Ticket
ADFA-3718
Observation
The
TemplateScrollGateKeeperautomatically resets its internal state if the layout width changes, which helps maintain accurate scroll detection across device orientation changes or resizing events.