Skip to content

refactor(dashboards-ng): make editable a single source of truth#1892

Open
chintankavathia wants to merge 1 commit intomainfrom
refactor/dashboards/remove-multiple-editable-states
Open

refactor(dashboards-ng): make editable a single source of truth#1892
chintankavathia wants to merge 1 commit intomainfrom
refactor/dashboards/remove-multiple-editable-states

Conversation

@chintankavathia
Copy link
Copy Markdown
Member

@chintankavathia chintankavathia commented Apr 17, 2026

Consolidates the dashboard grid's editable state by removing the duplicated editableInternal property in SiGridComponent and the editable$ BehaviorSubject in SiGridService. The editable model signal on SiGridComponent is now the sole owner of the editable state.

In SiWidgetHostComponent, the editable state is now passed as a regular input() binding instead of being consumed via an async observable from the grid service.

Why
Previously, the editable state was tracked in three places:

  • The editable model signal (public API / input)
  • A private editableInternal boolean in SiGridComponent
  • A BehaviorSubject (editable$) on SiGridService

This made the code harder to reason about—edit(), save(), and cancel() had to keep all three in sync, and SiWidgetHostComponent subscribed to the service observable rather than receiving the value directly from its parent.


Documentation.
Examples.
Dashboards Demo.
Playwright report.

Coverage Reports:

Code Coverage

@chintankavathia chintankavathia changed the title refactor(dashboards-ng): make editable as only single source of truth refactor(dashboards-ng): make editable a single source of truth Apr 17, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the dashboard grid's edit state management by replacing the BehaviorSubject in SiGridService and the internal editableInternal property with Angular signals and model inputs. While this modernizes the state handling, several regressions were identified where guards in the edit() and cancel() methods prevent necessary state transitions when triggered via input bindings. Additionally, feedback was provided to avoid any types in error handlers per the style guide and to ensure setupEditable is not called prematurely during the initial change detection cycle in SiWidgetHostComponent.

Comment thread projects/dashboards-ng/src/components/grid/si-grid.component.ts Outdated
Comment thread projects/dashboards-ng/src/components/grid/si-grid.component.ts Outdated
Comment thread projects/dashboards-ng/src/components/grid/si-grid.component.ts
Comment thread projects/dashboards-ng/src/components/widget-host/si-widget-host.component.ts Outdated
@chintankavathia chintankavathia force-pushed the refactor/dashboards/remove-multiple-editable-states branch 2 times, most recently from db919ab to a2d3a80 Compare April 17, 2026 05:24
@chintankavathia chintankavathia force-pushed the refactor/dashboards/remove-multiple-editable-states branch from a2d3a80 to e261eb1 Compare April 17, 2026 06:40
@chintankavathia chintankavathia marked this pull request as ready for review April 17, 2026 07:27
@chintankavathia chintankavathia requested review from a team as code owners April 17, 2026 07:27
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.

1 participant