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

[Dashboard] [Controls] Fix dashboard reset when initial state has no controls #159404

Merged
merged 2 commits into from Jun 9, 2023

Conversation

Heenawter
Copy link
Contributor

@Heenawter Heenawter commented Jun 9, 2023

Summary

Consider taking the following steps:

  1. Create a new dashboard and save it
  2. Add a control and, without saving,
  3. Reset the changes to the dashboard

Before

Before this PR, we were not updating the control group input if the lastSavedControlGroupInput was undefined (which only happens when a dashboard has never been saved with any controls and/or edits to the control group settings) - this caused a problem when trying to reset a dashboard from having controls back to the state where lastSavedControlGroupInput was undefined because the dashboard's input would get updated as expected (i.e. the dashboard would think it no longer has any controls), but the control group's input wouldn't get updated (i.e. the control group would think it still has controls).

Because of this discrepancy, the control would stick around until you refreshed the dashboard:

Screen.Recording.2023-06-09.at.1.52.41.PM.mov

After

Now, after this PR, I fixed this by resetting back to the default control group input if lastSavedControlGroupInput is undefined on reset:

Screen.Recording.2023-06-09.at.1.55.12.PM.mov

Checklist

For maintainers

@Heenawter Heenawter added bug Fixes for quality problems that affect the customer experience Feature:Dashboard Dashboard related features Feature:Input Control Input controls visualization Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:small Small Level of Effort release_note:skip Skip the PR/issue when compiling release notes impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Project:Controls labels Jun 9, 2023
@Heenawter Heenawter self-assigned this Jun 9, 2023
@Heenawter Heenawter added release_note:fix and removed release_note:skip Skip the PR/issue when compiling release notes labels Jun 9, 2023
@Heenawter Heenawter force-pushed the fix-no-controls-reset_2023-06-09 branch from a685464 to 8353235 Compare June 9, 2023 19:21
@Heenawter Heenawter marked this pull request as ready for review June 9, 2023 20:01
@Heenawter Heenawter requested a review from a team as a code owner June 9, 2023 20:01
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

Fix LGTM! Thanks for tackling this one. Code only review, but everything looks like it should work great.

@Heenawter Heenawter enabled auto-merge (squash) June 9, 2023 20:19
@Heenawter Heenawter merged commit 11c761f into elastic:main Jun 9, 2023
15 of 16 checks passed
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
dashboard 356.0KB 356.0KB +45.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 414 418 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 498 502 +4
total +6

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @Heenawter

@kibanamachine kibanamachine added v8.9.0 backport:skip This commit does not require backporting labels Jun 9, 2023
saarikabhasi pushed a commit to saarikabhasi/kibana that referenced this pull request Jun 14, 2023
…controls (elastic#159404)

## Summary

Consider taking the following steps:

1. Create a new dashboard and save it
2. Add a control and, without saving,
3. Reset the changes to the dashboard

### Before 
Before this PR, we were not updating the control group input if the
`lastSavedControlGroupInput` was `undefined` (which only happens when a
dashboard has **never** been saved with any controls and/or edits to the
control group settings) - this caused a problem when trying to reset a
dashboard from having controls back to the state where
`lastSavedControlGroupInput` was `undefined` because the **dashboard's**
input would get updated as expected (i.e. the dashboard would think it
no longer has any controls), but the control group's input wouldn't get
updated (i.e. the control group would think it **still has** controls).

Because of this discrepancy, the control would stick around until you
refreshed the dashboard:


https://github.com/elastic/kibana/assets/8698078/c9da58dc-3373-493d-9bba-5d2540c19560

### After
Now, after this PR, I fixed this by resetting back to the default
control group input if `lastSavedControlGroupInput` is `undefined` on
reset:


https://github.com/elastic/kibana/assets/8698078/e41838e6-6dbe-47a1-bea7-28f20eddcf80


### Checklist

- [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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting bug Fixes for quality problems that affect the customer experience 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:small Small Level of Effort Project:Controls release_note:fix Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants