Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Controls] Move control group diffing from Dashboard to Control plugin #174703

Closed
Heenawter opened this issue Jan 11, 2024 · 1 comment · Fixed by #175146
Closed

[Controls] Move control group diffing from Dashboard to Control plugin #174703

Heenawter opened this issue Jan 11, 2024 · 1 comment · Fixed by #175146
Assignees
Labels
Feature:Dashboard Dashboard related features Feature:Input Control Input controls visualization impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:large Large Level of Effort Project:Controls Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas

Comments

@Heenawter
Copy link
Contributor

Heenawter commented Jan 11, 2024

Describe the feature:
Currently, the Dashboard handles the diffing of all panels + the control group to determine whether or not there are any unsaved changes - however, as part of the Embeddable Refactor work, this will no longer be the case. Instead, the control group (and the dashboard children/panels) will handle their own diffing and simply communicate to Dashboard whether or not they have unsaved changes (and, on request, what these unsaved changes are for session storage management).

Describe a specific use case for the feature:
This is related to the Embeddable Refactor work, but it is also necessary for the Apply button because, in order for that to work, the control group needs to know when it has unsaved changes so that we can enable/disable the buttons accordingly.

@Heenawter Heenawter added Feature:Dashboard Dashboard related features Feature:Input Control Input controls visualization Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:large Large Level of Effort impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Project:Controls labels Jan 11, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@Heenawter Heenawter self-assigned this Jan 11, 2024
Heenawter added a commit that referenced this issue Jan 30, 2024
)

Closes #174703

## Summary

Currently, the burden of determining unsaved changes is handled entirely
by the Dashboard - this means that we need special logic for comparing
the state of every piece of a dashboard, including the controls and the
panels. Once the [embeddable
refactor](#167429) work is
complete, this will no longer be the case; instead, each component will
be responsible for handling its **own** unsaved changes, and the
Dashboard will simply listen for updates.

As an intermediate step, this PR separates out the control group logic
from the Dashboard plugin so that, when it comes time, the transition to
the new embeddable framework will be much smoother. This PR also
unblocks #170396 since, now that
the control group is responsible for handling its own unsaved changes, I
should be able to determine when to enable/disable the "reset changes"
button.


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: kibanamachine <[email protected]>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this issue Feb 15, 2024
…tic#175146)

Closes elastic#174703

## Summary

Currently, the burden of determining unsaved changes is handled entirely
by the Dashboard - this means that we need special logic for comparing
the state of every piece of a dashboard, including the controls and the
panels. Once the [embeddable
refactor](elastic#167429) work is
complete, this will no longer be the case; instead, each component will
be responsible for handling its **own** unsaved changes, and the
Dashboard will simply listen for updates.

As an intermediate step, this PR separates out the control group logic
from the Dashboard plugin so that, when it comes time, the transition to
the new embeddable framework will be much smoother. This PR also
unblocks elastic#170396 since, now that
the control group is responsible for handling its own unsaved changes, I
should be able to determine when to enable/disable the "reset changes"
button.


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: kibanamachine <[email protected]>
fkanout pushed a commit to fkanout/kibana that referenced this issue Mar 4, 2024
…tic#175146)

Closes elastic#174703

## Summary

Currently, the burden of determining unsaved changes is handled entirely
by the Dashboard - this means that we need special logic for comparing
the state of every piece of a dashboard, including the controls and the
panels. Once the [embeddable
refactor](elastic#167429) work is
complete, this will no longer be the case; instead, each component will
be responsible for handling its **own** unsaved changes, and the
Dashboard will simply listen for updates.

As an intermediate step, this PR separates out the control group logic
from the Dashboard plugin so that, when it comes time, the transition to
the new embeddable framework will be much smoother. This PR also
unblocks elastic#170396 since, now that
the control group is responsible for handling its own unsaved changes, I
should be able to determine when to enable/disable the "reset changes"
button.


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Dashboard Dashboard related features Feature:Input Control Input controls visualization impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:large Large Level of Effort Project:Controls Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants