Skip to content

ADFA-3235 | Fix checkbox selection bug in project list#1075

Open
jatezzz wants to merge 1 commit intostagefrom
fix/ADFA-3235-checkbox-listener
Open

ADFA-3235 | Fix checkbox selection bug in project list#1075
jatezzz wants to merge 1 commit intostagefrom
fix/ADFA-3235-checkbox-listener

Conversation

@jatezzz
Copy link
Collaborator

@jatezzz jatezzz commented Mar 13, 2026

Description

Replaced setOnCheckedChangeListener with setOnClickListener for the checkbox in DeleteProjectListAdapter. This resolves an issue where the RecyclerView view recycling process would programmatically trigger the listener during scrolling, causing projects to be incorrectly selected or deselected. Unused imports were also removed to clean up the file.

Details

Logic-related fix to ensure stable selection states when scrolling through the list of projects. No UI layout changes.

Before changes

document_5017221786907969367.mp4

After changes

document_5017221786907969356.mp4

Ticket

ADFA-3235

Observation

Using setOnClickListener ensures that the selection logic is only triggered by actual physical user interaction. This avoids the unintended behavior of OnCheckedChangeListener, which fires every time the isChecked property is updated programmatically when binding a recycled view.

Prevents unintended selection state changes during RecyclerView scrolling.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c1bafe91-3357-4f43-84f4-913265e01696

📥 Commits

Reviewing files that changed from the base of the PR and between deafbbd and 4c06090.

📒 Files selected for processing (1)
  • app/src/main/java/com/itsaky/androidide/adapters/DeleteProjectListAdapter.kt

📝 Walkthrough

Release Notes - Fix checkbox selection bug in project list

Changes:

  • Replaced setOnCheckedChangeListener with setOnClickListener on the checkbox in DeleteProjectListAdapter to fix RecyclerView view recycling causing unintended project selections/deselections during scrolling
  • Selection logic now manually updates selectedProjects by checking binding.checkbox.isChecked after the click event
  • Removed unused imports

Risks & Best Practices Violations:

  • ⚠️ Listener accumulation: New OnClickListener is registered every time bind() is called without removing the previous one. This creates stacking listeners on recycled views, leading to potential memory leaks and redundant callback executions. Recommendation: Call binding.checkbox.setOnClickListener(null) before setting the new listener, or use a single reusable listener instance.
  • ⚠️ State synchronization dependency: The implementation now relies on binding.checkbox.isChecked being updated immediately when clicked. This assumes Android's CheckBox implementation updates state synchronously before triggering click listeners—a valid but implicit dependency on framework behavior.
  • ✓ Functional fix is sound: Using click listeners prevents view binding from programmatically triggering selection logic, which correctly addresses the root cause.

Walkthrough

Refactors checkbox event handling in DeleteProjectListAdapter by replacing setOnCheckedChangeListener with setOnClickListener, moving selection state management logic directly into the click handler instead of relying on the listener callback pattern.

Changes

Cohort / File(s) Summary
Checkbox Event Handling Refactor
app/src/main/java/com/itsaky/androidide/adapters/DeleteProjectListAdapter.kt
Changed checkbox event listener from setOnCheckedChangeListener to setOnClickListener; selection state update logic now handled inline within the click handler based on checkbox.isChecked status.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • hal-eisen-adfa
  • dara-abijo-adfa

Poem

🐰 A checkbox clicked, no listener bound,
The handler now takes solid ground,
State shifts inline with every tap,
No callback gap—a cleaner map!
Projects selected, crisp and bright,
The refactor hops just right.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: fixing a checkbox selection bug in the project list, which directly matches the core issue addressed in the changeset.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, explaining the problem with setOnCheckedChangeListener, the solution using setOnClickListener, and the removal of unused imports.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/ADFA-3235-checkbox-listener
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

You can disable poems in the walkthrough.

Disable the reviews.poem setting to disable the poems in the walkthrough.

@jatezzz jatezzz requested a review from a team March 13, 2026 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant