fix!: decouple grid row details and selection from activeItem#9612
Merged
Conversation
vursen
changed the title
vursen
changed the title
vursen
force-pushed
the
fix-details-opened-logic
branch
from
c8e4742 to
d3d38f5
Compare
|
|
||
| if (selectionMode === 'SINGLE') { | ||
| if (item && item.selected && grid.__deselectDisallowed) { | ||
| return; |
Contributor
There was a problem hiding this comment.
The early return here prevents the details open / closing logic from running. Thus when __deselectDisallowed is active that means clicking on a selected row still prevents details from closing.
| }); | ||
|
|
||
| it('should not set details hidden on selected item click when deselect is disallowed', () => { | ||
| it('should set details hidden on selected item click when deselect is disallowed', () => { |
Contributor
There was a problem hiding this comment.
The clicked item doesn't appear to be selected, at least I can't find any test setup for it.
The test above also doesn't match its description.
Contributor
Author
There was a problem hiding this comment.
Fixed these tests to actually select items.
This was referenced Jun 30, 2026
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).
Return early from onItemActivate when the clicked row has no loaded item, so a click on a still-loading row no longer triggers selection or details toggling. Deselect the clicked item instead of all selected items, so the decision uses the item's current data rather than stale state.
The row model can keep a stale item while its data reloads, so the `!item` guard let clicks through during loading. Check the data provider cache instead, and read selection state from selectedKeys rather than the stale model item.
vursen
force-pushed
the
fix-details-opened-logic
branch
from
90c64d9 to
eba5d40
Compare
|
sissbruecker
approved these changes
Jul 1, 2026
sissbruecker
added a commit
to sissbruecker/flow-components
that referenced
this pull request
Jul 3, 2026
sissbruecker
added a commit
to sissbruecker/flow-components
that referenced
this pull request
Jul 3, 2026
sissbruecker
added a commit
to sissbruecker/flow-components
that referenced
this pull request
Jul 7, 2026
sissbruecker
added a commit
to sissbruecker/flow-components
that referenced
this pull request
Jul 7, 2026
sissbruecker
added a commit
to sissbruecker/flow-components
that referenced
this pull request
Jul 7, 2026
sissbruecker
added a commit
to sissbruecker/flow-components
that referenced
this pull request
Jul 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Row details and single-selection both relied on
activeItemproperty observers. Routing both throughactiveItemcaused several bugs, which the updated WTR tests now cover:activeItem, which prevented the details observer from running.activeItemchange triggered the details observer as if the row had been clicked.The PR replaces the two
activeItemobservers withcell-activateandrow-activatelisteners that readdetailsOpenedandselectedfrom the event. These events only fire on user interaction, so programmatic changes don't trigger them. There's also no longer any need to mirror selection ontoactiveItemwhen selection changes come from the server. As a result, details and selection are now fully independent ofactiveItemand only react to real clicks.Warning
This should be a relatively safe change, since rows were always rendered as selected via the
selectedattribute/part rather thanactive, and the same applies todetails-opened. That said, theactiveItemproperty is no longer populated, which could affect apps that did something based on it in their custom logic.