Skip to content

feat(list-builder-view-grids): implement Phase 1 and Phase 2 of state management refactoring#5

Open
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1768170539-state-management-refactoring
Open

feat(list-builder-view-grids): implement Phase 1 and Phase 2 of state management refactoring#5
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1768170539-state-management-refactoring

Conversation

@devin-ai-integration
Copy link
Copy Markdown

feat(list-builder-view-grids): implement state management refactoring Phase 1 & 2

Summary

This PR implements Phase 1 and Phase 2 of the state management refactoring for the list-builder-view-grids component. The refactoring addresses technical debt documented around lines 593-598 of list-view-grid.component.ts, where scan operators were used as workarounds for timing issues caused by the "state object observable with static properties" pattern.

Phase 1 - New State Architecture:

  • NewGridState: Uses individual BehaviorSubject instances for displayedColumns and columns properties, exposing them as separate observables
  • NewGridStateDispatcher: Handles actions and updates individual state properties through update methods

Phase 2 - New Component Implementation:

  • NewSkyListViewGridComponent: Uses the new state pattern with simplified getGridItems() and getSelectedIds() methods that don't require scan operators
  • NewSkyListColumnSelectorActionComponent: Uses combineLatest to subscribe to multiple observables from the new state pattern

All new components are created alongside existing deprecated components (prefixed with "New") for parallel deployment.

Review & Testing Checklist for Human

  • Verify timing fix: The core purpose is to eliminate timing issues where state updates were consumed out of order. Test scenarios where search text updates and list item updates occur in rapid succession to confirm the new pattern resolves this.
  • Module registration: The new components are NOT yet added to any Angular module. They need to be added to declarations/exports before they can be used.
  • Test the column selector: Verify openColumnSelector() works correctly with the new combineLatest pattern - ensure both columns and displayedColumns are retrieved atomically.
  • Verify no race conditions: Confirm that removing scan operators from getGridItems() and getSelectedIds() doesn't introduce new race conditions.
  • Add unit tests: Consider adding test coverage for the new components before merging.

Recommended test plan:

  1. Add the new components to the module
  2. Create a test page using sky-new-list-view-grid with multiple columns
  3. Perform rapid search operations while columns are loading
  4. Verify column selection/deselection works correctly
  5. Test the column chooser modal functionality

Notes

  • The new components are marked as @internal for the parallel deployment phase
  • The original SkyLogService deprecation logging was intentionally omitted from the new component
  • Local tests could not run due to ChromeHeadless environment issues - CI will validate

Link to Devin run: https://app.devin.ai/sessions/424126bd0147451dadd1c68fba8fdcef
Requested by: @bcmake

… management refactoring

Phase 1: Create New State Architecture Foundation
- NewGridState: Uses individual BehaviorSubjects for displayedColumns and columns
- NewGridStateDispatcher: Handles actions and updates individual state properties

Phase 2: Create New Component Implementation
- NewSkyListViewGridComponent: Uses new state pattern without scan operators
- NewSkyListColumnSelectorActionComponent: Uses combineLatest for multiple observables

The new implementation eliminates timing issues where state updates were consumed
out of order by using a 'static state object with observable properties' pattern
instead of the previous 'state object observable with static properties' pattern.

Co-Authored-By: benc@cognition.ai <Benc@windsurf.com>
@devin-ai-integration
Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

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.

0 participants