Skip to content

[AI review experiment] upstream PR #9612#3

Closed
sissbruecker wants to merge 9 commits into
mainfrom
ai-review-pr-9612
Closed

[AI review experiment] upstream PR #9612#3
sissbruecker wants to merge 9 commits into
mainfrom
ai-review-pr-9612

Conversation

@sissbruecker

Copy link
Copy Markdown
Owner

Throwaway PR for AI code-review experiment.

Reconstructs the pre-review state of vaadin#9612 at commit 084d19abcb892744ae56ca4f069f1a078cc25381.

Do not merge.

vursen and others added 9 commits June 25, 2026 14:18
Drive row details and single-selection from the grid's cell-activate
and row-activate events instead of two activeItem property observers.
Reading detailsOpened and selected straight from the activated item
decouples details from selection and fixes three cases:

- Details now close when clicking a row whose details are open even
  if deselect is disallowed; before, reverting activeItem stopped the
  details observer from running.
- Selecting an item from server data no longer opens its details;
  before, the programmatic activeItem change triggered the observer.
- Clicking a row with details opened from data now closes them in a
  single click.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ven in tests — from clicking the first cell to dispatching the `row-activate` event (the keyboard activation path), plus new tests for Space-key row selection.

test: drive grid single-selection via row-activate event

- decouple testbench select/deselect from cell clicks so it no
  longer triggers item-click listeners as a side effect
- add tests for selecting and deselecting a row with the Space key
`destroyData` removed the item from the visible-details set, so
details closed after the item left and re-entered the DOM (parent
row collapse/expand, or scrolling out of and back into view).
@sissbruecker

Copy link
Copy Markdown
Owner Author

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Logic Bug

In single-selection mode, activating an already-selected item while __deselectDisallowed is true returns from the handler before row-details handling runs. This means clicking an open details row in that state never calls setDetailsVisible(null), so details can get stuck open. Skip only the deselection path and continue to the details toggle.

if (item && item.selected && grid.__deselectDisallowed) {
  return;
Nested Events

The new activation listeners handle any cell-activate or row-activate event that reaches the grid. If another grid is rendered inside this grid's details, activation events from the nested grid can bubble to the parent and cause parent-side selection or details updates using the child item key. Guard the handler so it only processes events whose target is this grid.

function onItemActivate(event) {
  const item = event.detail.model?.item;

  if (selectionMode === 'SINGLE') {
    if (item && item.selected && grid.__deselectDisallowed) {
      return;
    }

    if (item && !item.selected) {
      grid.$connector.doSelection([item], true);
    } else {
      grid.$connector.doDeselection([...grid.selectedItems], true);
    }
  }

  if (!grid.__disallowDetailsOnClick) {
    if (item && !item.detailsOpened) {
      grid.$server.setDetailsVisible(item.key);
    } else {
      grid.$server.setDetailsVisible(null);
    }
  }
}

grid.addEventListener('cell-activate', onItemActivate);
grid.addEventListener('row-activate', onItemActivate);

@sissbruecker
sissbruecker deleted the ai-review-pr-9612 branch June 30, 2026 08:21
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