fix(ui): prevent directory picker folder clicks selecting#986
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
📝 WalkthroughWalkthroughRefactors directory-picker row activation so the outer Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
7a74573 to
42a230f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/app/shared/components/directory-picker/directory-picker.component.html (1)
138-167:⚠️ Potential issue | 🟠 MajorAdd semantic role and accessible naming for the interactive row and checkbox.
The
div.directory-itemat line 141 hastabindex="0"for keyboard interaction but is missing a semantic role and accessible name. Thep-checkboxat line 150 lacks an[ariaLabel]binding. These omissions create an accessibility gap for screen-reader users navigating the folder list.♿ Suggested fix
<div class="directory-item" [class.selected]="isFolderSelected(folder)" tabindex="0" + role="button" + [attr.aria-label]="getFolderName(folder)" (click)="onRowClick(folder)" (keydown.enter)="onRowClick(folder)" (keydown.space)="onRowClick(folder)" > <div class="directory-checkbox"> <p-checkbox [inputId]="'folder-' + folder" + [ariaLabel]="getFolderName(folder)" [binary]="true" [(ngModel)]="selectedFoldersMap[folder]" (ngModelChange)="onCheckboxChange(folder, $event)" (click)="$event.stopPropagation()" (keydown.enter)="$event.stopPropagation()" (keydown.space)="$event.stopPropagation()" /> </div>Additionally, the
(keydown.space)handler on the focusabledivshould call$event.preventDefault()to suppress the browser's default page-scroll behavior during Space activation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/shared/components/directory-picker/directory-picker.component.html` around lines 138 - 167, The interactive folder row (div.directory-item) lacks a semantic role and accessible name and its space key handler doesn't prevent default scrolling; update the element used by isFolderSelected(folder) and onRowClick(folder) to include role="button" and an aria-label that uses getFolderName(folder) (e.g., "Open folder {getFolderName(folder)}" or include the folder path) and change the (keydown.space) to call $event.preventDefault() before invoking onRowClick(folder). Also add an accessible label to the p-checkbox used with selectedFoldersMap[folder] and onCheckboxChange(folder, $event) by binding its ariaLabel (e.g., [ariaLabel]="'Select ' + getFolderName(folder)"), and ensure the checkbox click/keydown handlers still stop propagation. Ensure references remain to isFolderSelected, onRowClick, selectedFoldersMap, onCheckboxChange, and getFolderName when making these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/src/app/shared/components/directory-picker/directory-picker.component.html`:
- Line 144: The Space key handler on the focusable row currently calls
onRowClick(folder) but doesn't prevent the browser's default scrolling; update
the template binding for (keydown.space) to call $event.preventDefault() before
invoking onRowClick(folder) so the page doesn't scroll when activating a row;
locate the (keydown.space)="onRowClick(folder)" binding in
directory-picker.component.html and change it to call $event.preventDefault()
then onRowClick(folder).
---
Outside diff comments:
In
`@frontend/src/app/shared/components/directory-picker/directory-picker.component.html`:
- Around line 138-167: The interactive folder row (div.directory-item) lacks a
semantic role and accessible name and its space key handler doesn't prevent
default scrolling; update the element used by isFolderSelected(folder) and
onRowClick(folder) to include role="button" and an aria-label that uses
getFolderName(folder) (e.g., "Open folder {getFolderName(folder)}" or include
the folder path) and change the (keydown.space) to call $event.preventDefault()
before invoking onRowClick(folder). Also add an accessible label to the
p-checkbox used with selectedFoldersMap[folder] and onCheckboxChange(folder,
$event) by binding its ariaLabel (e.g., [ariaLabel]="'Select ' +
getFolderName(folder)"), and ensure the checkbox click/keydown handlers still
stop propagation. Ensure references remain to isFolderSelected, onRowClick,
selectedFoldersMap, onCheckboxChange, and getFolderName when making these
changes.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: aabdcfa4-086f-4e93-99dd-64b416321efd
📒 Files selected for processing (2)
frontend/src/app/shared/components/directory-picker/directory-picker.component.htmlfrontend/src/app/shared/components/directory-picker/directory-picker.component.scss
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
frontend/src/**/*.{ts,tsx,html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation in TypeScript, HTML, and SCSS in frontend code
Files:
frontend/src/app/shared/components/directory-picker/directory-picker.component.scssfrontend/src/app/shared/components/directory-picker/directory-picker.component.html
frontend/src/**/*.{ts,tsx,html}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/src/**/*.{ts,tsx,html}: Follow frontend/eslint.config.js: component selectors use app-, directive selectors use app, and any is disallowed in frontend code
Put user-facing strings in Transloco files under frontend/src/i18n/
Files:
frontend/src/app/shared/components/directory-picker/directory-picker.component.html
🔇 Additional comments (1)
frontend/src/app/shared/components/directory-picker/directory-picker.component.scss (1)
373-377: Focus-visible styling is a good match for the row-level interaction change.This improves keyboard discoverability after making the whole row focusable/clickable.
only clicks on the checkbox should select the folder, clicking the rest of the row should lead to the user entering the folder
42a230f to
610d644
Compare
Description
clicking the directory picker rows causes the picker to select those rows while entering them. only clicks on the checkbox should select the folder, clicking the rest of the row should lead to the user entering the folder
this prevents the folder text from activating the checkbox and moves the row selector to the row element - now clicking the checkbox selects, and everything else enters
Screencast.From.2026-04-29.00-49-56.mp4
Linked Issue: Fixes #985
Changes
onRowClickcall to the row itselflabelto adiv+ removesforso clicking it doesn't activate the checkboxSummary by CodeRabbit
Bug Fixes
Style