Conversation
…e#558) When a ListView row has no route, the inner wrapper rendered a real <button> element. Cell slot content like Select/Dropdown triggers are themselves <button>s, so the browser re-parents the DOM on parse, breaking the inner trigger's popup events once a Dialog focus-scope wraps the list. Render a <div> with role=button/tabindex and explicit keydown handling instead, preserving keyboard accessibility without nesting buttons.
Confidence Score: 4/5The PR appears safe to merge, with a non-blocking request for regression coverage. The markup fix is coherent, but no automated test protects the Dialog popup reproduction or replacement keyboard behavior. Files Needing Attention: experimental/ListView/ListRow.vue Prompt To Fix All With AI### Issue 1
experimental/ListView/ListRow.vue:38-47
**Interaction regression lacks coverage**
Add a focused test covering the Dialog popup reproduction and the replacement Enter/Space behavior; without one, future row-wrapper changes can silently restore invalid nested buttons or break keyboard activation.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(ListView): stop nesting button eleme..." | Re-trigger Greptile |
| :class=" | ||
| list.options.getRowRoute ? 'contents' : '[all:unset] hover:[all:unset]' | ||
| " | ||
| @keydown.enter.self=" | ||
| !list.options.getRowRoute && !row.disabled && onRowClick($event) | ||
| " | ||
| @keydown.space.prevent.self=" | ||
| !list.options.getRowRoute && !row.disabled && onRowClick($event) | ||
| " | ||
| > |
There was a problem hiding this comment.
Interaction regression lacks coverage
Add a focused test covering the Dialog popup reproduction and the replacement Enter/Space behavior; without one, future row-wrapper changes can silently restore invalid nested buttons or break keyboard activation.
Context Used: Guidelines for reviewing Frappe Framework applicat... (source)
Knowledge Base Used: Restore synchronous dropdown item actions
Prompt To Fix With AI
This is a comment left during a code review.
Path: experimental/ListView/ListRow.vue
Line: 38-47
Comment:
**Interaction regression lacks coverage**
Add a focused test covering the Dialog popup reproduction and the replacement Enter/Space behavior; without one, future row-wrapper changes can silently restore invalid nested buttons or break keyboard activation.
**Context Used:** Guidelines for reviewing Frappe Framework applicat... ([source](https://github.com/frappe/skills/blob/main/skills/quality-code-review/SKILL.md))
**Knowledge Base Used:** [Restore synchronous dropdown item actions](https://app.greptile.com/frappe/-/custom-context/knowledge-base/frappe/frappe-ui/-/reverts/revert_566-20260130-dropdown-click-lifecycle-ee7076a.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Problem
Closes #558
A
ListViewrow with no route wraps its cell content in a real<button>element (ListRow.vuehad:is="getRowRoute ? 'div' : 'button'"). Select, Dropdown, and Combobox triggers are themselves<button>s, so rendering one inside a ListView cell produces a nested<button>— invalid HTML.Browsers re-parent nested buttons on parse, silently breaking the inner trigger's event wiring. The failure becomes loud as soon as the ListView is placed inside a
Dialog: reka-ui'sFocusScopeintercepts the mangled DOM and the inner popup never opens — the reporter's exact reproduction in #558.Fix
Always render the inner row wrapper as a
<div>:getRowRoute: unchanged —display: contentsdiv.<div role="button">withtabindex="0"and explicitkeydown.enter/keydown.spacehandlers that callonRowClick, preserving the keyboard accessibility a<button>gave the row without nesting buttons.Verification
NODE_ENV=test npx vitest run— 1574 tests pass (2 pre-existing editor timeouts onmainare unrelated).npx vue-tsc --noEmit -p tsconfig.app.json— clean.Not a public API change — the row's rendered classes, click handling, and hover/active semantics are unchanged.
Coverage: 72.50% (±0.00% vs
main)