Skip to content

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

Merged
jatezzz merged 3 commits intostagefrom
fix/ADFA-3235-checkbox-listener
Mar 17, 2026
Merged

ADFA-3235 | Fix checkbox selection bug in project list#1075
jatezzz merged 3 commits 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.

@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: 53eb1919-fa44-4d73-8642-d08ccde962f1

📥 Commits

Reviewing files that changed from the base of the PR and between 55be607 and 2c43be6.

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

📝 Walkthrough
  • Replaced checkbox setOnCheckedChangeListener with click-driven selection in DeleteProjectListAdapter:
    • Adapter now uses List<Checkable>; selection state is stored in each Checkable.
    • ViewHolder bind() sets checkbox.isChecked from item.isChecked and toggles item.isChecked when root is clicked.
    • Checkbox click delegates to root.performClick(); long-press on checkbox still calls onCheckboxLongPress().
    • Removed internal selectedProjects cache; getSelectedProjects() derives selection from items' isChecked.
    • updateProjects(...) signature updated to accept List<Checkable>; adapter.notifyDataSetChanged() used on updates.
  • DeleteProjectFragment updated to wrap projects in Checkable preserving previously selected paths when the list refreshes.
  • No UI/layout changes; removed unused imports. Before/after screenshots attached; linked ticket ADFA-3235.

Risks & Best-practice notes:

  • ⚠️ Implicit UI-state dependency: The implementation relies on synchronously updating checkbox state via root click and binding.checkbox.isChecked. While stable in practice, it still depends on click ordering; explicitly toggling state in code (as done here) is good, but be mindful if the view hierarchy or input handling changes.
  • ⚠️ Listener allocation on bind(): setOnClickListener/setOnLongClickListener are set inside bind(), causing per-bind reassignments. Prefer installing listeners once in the ViewHolder constructor and using the bound item/reference to avoid repeated allocations and potential subtle bugs with stale positions.
  • ⚠️ Full-list invalidation: updateProjects(...) calls notifyDataSetChanged(), which disables fine-grained animations and can be inefficient for large lists. Consider using DiffUtil to apply minimal updates.
  • ✓ Correctness improvement: Replacing OnCheckedChangeListener avoids RecyclerView view-recycling from triggering selection changes during programmatic checkbox state binding, preventing incorrect selections/deselections while scrolling.

Walkthrough

Changed the projects list to use Checkable<ProjectFile> throughout, moving selection state into each item; adapter now reads/writes isChecked on items and derives selected projects from that state. Fragment builds Checkable wrappers from project list and selected paths and updates the adapter accordingly.

Changes

Cohort / File(s) Summary
Adapter: selection model & UI binding
app/src/main/java/com/itsaky/androidide/adapters/DeleteProjectListAdapter.kt
Replaced internal selectedProjects cache with List<Checkable<ProjectFile>>; bind now accepts Checkable<ProjectFile>, uses item.data for display, toggles item.isChecked on root click, delegates checkbox clicks to root click, and getSelectedProjects() derives selected items from isChecked. Imports adjusted.
Fragment: build & supply Checkable list
app/src/main/java/com/itsaky/androidide/fragments/DeleteProjectFragment.kt
Wraps projects into Checkable<ProjectFile> using adapter selections (selectedPaths), initializes/updates adapter with checkable list, adjusts long-press tooltip anchor to binding.listProjects, and updates button-state checks to use adapter?.getSelectedProjects()?.isNotEmpty() == true.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • hal-eisen-adfa

Poem

🐰 In a burrow of bytes I nudge each line,

Checkmarks hop on rows, tidy and fine.
Wrapped in a shell, each project I keep,
Toggled by paws that never sleep.
A small refactor — a happy leap.

🚥 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 directly addresses the main change: fixing a checkbox selection bug in the project list by replacing the listener pattern.
Description check ✅ Passed The description clearly explains the fix by replacing setOnCheckedChangeListener with setOnClickListener, the problem it solves, and includes supporting details and screenshots.

✏️ 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

CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.

OpenGrep is compatible with Semgrep configurations. Add an opengrep.yml or semgrep.yml configuration file to your project to enable OpenGrep analysis.

Prevents unintended selection state changes during RecyclerView scrolling.
@jatezzz jatezzz force-pushed the fix/ADFA-3235-checkbox-listener branch from 53d5820 to 55be607 Compare March 16, 2026 15:26
…selection model

improve UX by toggling selection from item click and removing internal selection state
@jatezzz jatezzz requested a review from itsaky-adfa March 16, 2026 16:55
@jatezzz jatezzz merged commit 1061433 into stage Mar 17, 2026
2 checks passed
@jatezzz jatezzz deleted the fix/ADFA-3235-checkbox-listener branch March 17, 2026 17:40
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.

2 participants