feat(ag-grid-angular-theme): added row (row group) state and store directives (#DS-5046) - #195
Merged
Conversation
…rectives (#DS-5046)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds persistent row selection, row focus, and row-group state using local storage or URL query parameters. The changes include Angular directives, public exports, development routes and pages, reset controls, unit tests, and Playwright coverage. ChangesAG Grid state persistence
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Grid as ag-grid-angular
participant State as State directive
participant Store as LocalStorage or query-param store
User->>Grid: Select row or focus cell
Grid->>State: Emit selectionChanged or cellFocused
State->>Store: Persist state
User->>Grid: Reload page
Grid->>State: Emit firstDataRendered
State->>Store: Read persisted state
Store-->>State: Return row or focus state
State->>Grid: Restore selection or focused cell
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Visit the preview URL for this PR (updated for commit 1915407): https://data-grid-next--data-grid-pr-195-qrdwj7r8.web.app (expires Thu, 23 Jul 2026 12:15:30 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: b9d49913f5b5988e9af8690a8b37f16143707448 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR expands @koobiq/ag-grid-angular-theme with new persistence directives (row selection + focused cell) and adds persistence options to KbqAgGridRowGroup (collapsed group paths + group-aware selection), with accompanying unit + Playwright coverage and dev demo routes.
Changes:
- Added
KbqAgGridRowSelectionStateandKbqAgGridRowFocusStatedirectives with LocalStorage and query-param store implementations. - Extended
KbqAgGridRowGroupwith persisted collapsed/expanded state and persisted group-aware row selection state. - Added Jest specs + Playwright E2E scenarios and dev demo pages/routes for the new features.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ag-grid-angular-theme/tests/row-selection-state.ng.spec.ts | Unit tests for the new row-selection persistence directive. |
| packages/ag-grid-angular-theme/tests/row-focus-state.ng.spec.ts | Unit tests for the new focused-cell persistence directive. |
| packages/ag-grid-angular-theme/tests/row-group.ng.spec.ts | Added unit tests for row-group collapsed/selection persistence behaviors. |
| packages/ag-grid-angular-theme/src/row-selection-state.ng.ts | New directive + stores for persisting/restoring AG Grid row selection. |
| packages/ag-grid-angular-theme/src/row-focus-state.ng.ts | New directive + stores for persisting/restoring AG Grid focused cell. |
| packages/ag-grid-angular-theme/src/row-group.ng.ts | Added collapsed-path and selection persistence (stores, inputs, restore/save wiring). |
| packages/ag-grid-angular-theme/src/module.ng.ts | Exported the new directives via the theme module. |
| packages/ag-grid-angular-theme/index.ts | Exported the new directives/stores from the public API. |
| dev/ag-grid-angular/src/tests/row-selection-state.playwright-spec.ts | E2E coverage for row-selection persistence (localStorage + query params). |
| dev/ag-grid-angular/src/tests/row-selection-state.ng.ts | Dev demo components for row-selection persistence. |
| dev/ag-grid-angular/src/tests/row-focus-state.playwright-spec.ts | E2E coverage for focused-cell persistence (localStorage + query params). |
| dev/ag-grid-angular/src/tests/row-focus-state.ng.ts | Dev demo components for focused-cell persistence. |
| dev/ag-grid-angular/src/tests/row-group.ts | Dev row-group demo updated to wire collapsed + selection persistence and reset UI. |
| dev/ag-grid-angular/src/tests/row-group.playwright-spec.ts | E2E coverage for row-group collapsed + selection persistence. |
| dev/ag-grid-angular/src/main.ts | Added dev routes for the new persistence demos. |
|
Note Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
dev/ag-grid-angular/src/tests/row-selection-state.ng.ts (1)
59-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTemplate-bound members should be
protectedin both dev components. As per coding guidelines: "Useprotectedfor template bindings."
dev/ag-grid-angular/src/tests/row-selection-state.ng.ts#L59-L66: changestore,stateKey,columnDefs,rowSelection,getRowId,rowDatatoprotected.dev/ag-grid-angular/src/tests/row-focus-state.ng.ts#L57-L64: changestore,stateKey,columnDefs,defaultColDef,getRowId,rowDatatoprotected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dev/ag-grid-angular/src/tests/row-selection-state.ng.ts` around lines 59 - 66, Change the template-bound members in DevRowSelectionState to protected: rowData, store, stateKey, columnDefs, rowSelection, and getRowId. Apply the same visibility change in DevRowFocusState for rowData, store, stateKey, columnDefs, defaultColDef, and getRowId; no other behavior changes are needed.Source: Coding guidelines
packages/ag-grid-angular-theme/src/row-selection-state.ng.ts (1)
20-21: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRoute
windowaccess through an injectable token (addresses the existingKBQ_WINDOWTODOs). All four stores readwindow.localStorage/window.locationdirectly, which is not resolvable under SSR/non-browser rendering and hinders testability. Injecting aKBQ_WINDOW-style token (as the TODOs note) fixes this uniformly. I can open an issue or draft the token wiring if helpful.
packages/ag-grid-angular-theme/src/row-selection-state.ng.ts#L20-L21: replacewindow.localStoragewith the injected window token.packages/ag-grid-angular-theme/src/row-selection-state.ng.ts#L56-L57: replacewindow.locationwith the injected window token.packages/ag-grid-angular-theme/src/row-focus-state.ng.ts#L26-L27: replacewindow.localStoragewith the injected window token.packages/ag-grid-angular-theme/src/row-focus-state.ng.ts#L62-L63: replacewindow.locationwith the injected window token.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ag-grid-angular-theme/src/row-selection-state.ng.ts` around lines 20 - 21, Replace direct browser window access with an injectable KBQ_WINDOW-style token across the affected stores: update row-selection-state.ng.ts lines 20-21 and 56-57, and row-focus-state.ng.ts lines 26-27 and 62-63, injecting and reusing the token for localStorage and location access while preserving existing store behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dev/ag-grid-angular/src/tests/row-focus-state.ng.ts`:
- Line 76: Update the animateRows binding in the affected row-focus state test
to use Angular property binding with [animateRows]="false" instead of the
literal string attribute, matching the existing DevRowFocusState fix.
- Line 40: Update the animateRows usage in the row-focus test fixture to
property-bind the boolean value with [animateRows]="false", ensuring animations
are actually disabled instead of passing the truthy string "false".
In `@dev/ag-grid-angular/src/tests/row-selection-state.ng.ts`:
- Line 78: Update the animateRows binding in the row-selection state test to use
Angular property binding with a boolean false value instead of the string
literal, matching the expected behavior in DevRowSelectionState.
---
Nitpick comments:
In `@dev/ag-grid-angular/src/tests/row-selection-state.ng.ts`:
- Around line 59-66: Change the template-bound members in DevRowSelectionState
to protected: rowData, store, stateKey, columnDefs, rowSelection, and getRowId.
Apply the same visibility change in DevRowFocusState for rowData, store,
stateKey, columnDefs, defaultColDef, and getRowId; no other behavior changes are
needed.
In `@packages/ag-grid-angular-theme/src/row-selection-state.ng.ts`:
- Around line 20-21: Replace direct browser window access with an injectable
KBQ_WINDOW-style token across the affected stores: update
row-selection-state.ng.ts lines 20-21 and 56-57, and row-focus-state.ng.ts lines
26-27 and 62-63, injecting and reusing the token for localStorage and location
access while preserving existing store behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8a0c4d90-b797-4f69-b1b4-f2ab90effaac
📒 Files selected for processing (15)
dev/ag-grid-angular/src/main.tsdev/ag-grid-angular/src/tests/row-focus-state.ng.tsdev/ag-grid-angular/src/tests/row-focus-state.playwright-spec.tsdev/ag-grid-angular/src/tests/row-group.playwright-spec.tsdev/ag-grid-angular/src/tests/row-group.tsdev/ag-grid-angular/src/tests/row-selection-state.ng.tsdev/ag-grid-angular/src/tests/row-selection-state.playwright-spec.tspackages/ag-grid-angular-theme/index.tspackages/ag-grid-angular-theme/src/module.ng.tspackages/ag-grid-angular-theme/src/row-focus-state.ng.tspackages/ag-grid-angular-theme/src/row-group.ng.tspackages/ag-grid-angular-theme/src/row-selection-state.ng.tspackages/ag-grid-angular-theme/tests/row-focus-state.ng.spec.tspackages/ag-grid-angular-theme/tests/row-group.ng.spec.tspackages/ag-grid-angular-theme/tests/row-selection-state.ng.spec.ts
|
✅ Created PR with unit tests: #196 |
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
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.
Summary by CodeRabbit
New Features
Tests