Refactor to reuse logic from ChangeTracker #87417
Conversation
ivanortegaalba
requested review from
axelavargas and
dprokop
and removed request for
a team and
axelavargas
| this.setState({ isDirty: diffCount > 0 }); | ||
| private _onActivate() { | ||
| this.loadDataSource(); | ||
| const changesSub = this.subscribeToEvent(SceneObjectStateChangedEvent, this._handleStateChange); |
There was a problem hiding this comment.
Since the events bubble up, we don't need to subscribe to any children, we can subscribe to the VizPanelManager
| }, 250); | ||
|
|
||
| private _handleStateChange = (event: SceneObjectStateChangedEvent) => { | ||
| if (!Object.prototype.hasOwnProperty.call(event.payload.partialUpdate, 'data')) { | ||
| this._updateDirty(); | ||
| if (DashboardSceneChangeTracker.isUpdatingPersistedState(event)) { |
There was a problem hiding this comment.
I added it as static, to keep all the logic about what changes affects the persisted state in a single place
There was a problem hiding this comment.
Looks good, but I found one thing that's a bit confusing. This applies when a new panel is added:
- Add panel to dashboard
- Discard changes and Save buttons are active.
- Click Discard and the panel is still added to a dashboard.
Similarly, when refreshing page with panel edit active Save button is active (migrations), but the Discard is deactivated.
This is a bit confusing, not sure what we can do about it.
| private _detectPanelModelChanges = debounce(() => { | ||
| const { hasChanges } = getPanelChanges( | ||
| vizPanelToPanel(this.state.sourcePanel.resolve()), | ||
| vizPanelToPanel(this.state.panel) | ||
| ); | ||
| this.setState({ isDirty: hasChanges }); |
There was a problem hiding this comment.
Should we add some tests for this?
There was a problem hiding this comment.
Yes, I agree. This was a poc PR on Oscar's PR to approach the issue, I'll add those test in the original PR 😄
This is a great finding Dominik, I'll solve this in Oscar's PR. IMO, the discard should not add the panel to the dashboard 👍
So, I'd not focus on solving this issue of enabling the save button, because we don't really have a way to avoid it. Unless we don't display the save dashboard when editing a panel. |
ivanortegaalba
added
no-backport
ivanortegaalba
merged commit 8a3417e
into
oscark/implement-discard-panel-changes-disable-and-enable
No description provided.