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] Add reset button #154872

Merged
merged 16 commits into from
Apr 21, 2023

Conversation

Heenawter
Copy link
Contributor

@Heenawter Heenawter commented Apr 12, 2023

Closes #132457

Summary

This PR adds a reset button to the dashboard top navigation in both edit and view mode - when clicked, if the dashboard has unsaved changes, it will revert the dashboard to the last saved state:

Screen.Recording.2023-04-18.at.4.24.27.PM.mov

Note
The above video contains some old copy for the modals. Please refer to "All copy changes" below to see the updated copy.

Note that, by adding more buttons to the top nav bar, we are increasing the risk of someone hitting this accessibility issue (where the breadcrumbs get completely overlapped before the browser is small enough for the navigation to collapse) - this will either be addressed on the Shared UX side or will be addressed as part of our nav bar redesign 👍

All Copy Changes

  • Listing Page

    Before After
    Screenshot 2023-04-18 at 4 29 10 PM image
    image image
  • Dashboard - Edit Mode

    Before After
    image image
  • Dashboard - View Mode

    Before After
    N/A since you couldn't discard changes from view mode previously image

Flaky Test Runner

  • test/functional/apps/dashboard/group1/dashboard_unsaved_state.ts
  • test/functional/apps/dashboard_elements/controls/options_list/options_list_dashboard_interaction.ts

Checklist

For maintainers

@Heenawter Heenawter added release_note:enhancement Feature:Dashboard Dashboard related features Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:medium Medium Level of Effort impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. labels Apr 12, 2023
@Heenawter Heenawter self-assigned this Apr 12, 2023
@Heenawter Heenawter force-pushed the dashboard-reset-button_2023-04-12 branch 8 times, most recently from 895d062 to c24ceb5 Compare April 13, 2023 22:31
@Heenawter Heenawter force-pushed the dashboard-reset-button_2023-04-12 branch 2 times, most recently from e69b59d to 3bbdf18 Compare April 17, 2023 19:57
state.explicitInput = state.componentState.lastSavedInput;
state.explicitInput = {
...state.componentState.lastSavedInput,
viewMode: state.explicitInput.viewMode, // keep current view mode when resetting
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is possible for a view view mode to be included in a dashboard's saved input - for example, in the sample dashboards, their imported state is in view mode which meant that hitting the "reset" would actually flip them from edit mode into view mode (since this is included as part of the last saved input). This line ensures that view mode is ignored when resetting to the last saved input.

Comment on lines +132 to +135
onClose: (overlayRef) => {
if (overlayTracker) overlayTracker.clearOverlays();
overlayRef.close();
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes a small bug I discovered with the panel settings flyout where, if you closed the flyout by clicking outside of the flyout rather than via the "Close" buttons, hasOverlays remains true and so a bunch of items in the top nav bar stayed disabled.

Before

Apr-13-2023 15-28-16

After

Apr-13-2023 15-27-09

@Heenawter Heenawter force-pushed the dashboard-reset-button_2023-04-12 branch 3 times, most recently from 717d39c to 48a1c40 Compare April 18, 2023 15:21
@Heenawter Heenawter force-pushed the dashboard-reset-button_2023-04-12 branch from 1f33bb3 to 322378b Compare April 18, 2023 15:21
@Heenawter Heenawter force-pushed the dashboard-reset-button_2023-04-12 branch from f4d62fe to 9c6c5ac Compare April 18, 2023 22:27
@gchaps
Copy link
Contributor

gchaps commented Apr 19, 2023

Some minor edits to the 2 Reset modals.

Listing page and Dashboard edit mode

Title: Reset dashboard?
Description: All unsaved changes will be lost.
Button: Reset dashboard

Dashboard view mode

Title: Reset dashboard?
Description: This dashboard will return to tis last saved state. You might lose changes to filters and queries.
Button: Reset dashboard

@gchaps gchaps self-assigned this Apr 19, 2023
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #38 / cases security and spaces enabled: trial Common delete_cases alerts security_solution removes multiple cases from the alert schema when deleting multiple cases

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 433.3KB 433.9KB +584.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
embeddable 75.7KB 75.7KB +44.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 394 397 +3

Total ESLint disabled count

id before after diff
securitySolution 474 477 +3

History

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

cc @Heenawter @gchaps

defaultMessage: `All unsaved changes will be lost.`,
})
: i18n.translate('dashboard.resetChangesConfirmModal.resetChangesDescription', {
defaultMessage: `This dashboard will return to its last saved state. You might lose changes to filters and queries.`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cqliu1 Brought up a good point that this could be rephrased to be something like "This dashboard will return to its last saved state. You might lose changes to things like filters and queries." to make it less explicit - after all, you could lose changes to control selections (which kinda fall in to the "filters" bucket) or maps zoom/positioning, but it's hard to call out those things specifically without potentially causing confusion.

Not sure we want to hold up this PR on a small copy change, but throwing it out there 💃

@Heenawter Heenawter marked this pull request as ready for review April 20, 2023 18:00
@Heenawter Heenawter requested a review from a team as a code owner April 20, 2023 18:00
@elasticmachine
Copy link
Contributor

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

@jsanz
Copy link
Member

jsanz commented Apr 21, 2023

I like the new copy changes, they are concise and better to understand. Great work 👏

@ThomThomson ThomThomson self-requested a review April 21, 2023 14:42
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.

Everything LGTM! Tested functionality in Chrome and looked through code. Nice functional test changes, and glad to see the copy changes to standardize us on reset rather than discard.

@botelastic botelastic bot added the Feature:Embedding Embedding content via iFrame label Apr 21, 2023
@Heenawter Heenawter merged commit 29ffa17 into elastic:main Apr 21, 2023
@Heenawter Heenawter deleted the dashboard-reset-button_2023-04-12 branch April 21, 2023 15:30
@kibanamachine kibanamachine added v8.8.0 backport:skip This commit does not require backporting labels Apr 21, 2023
kpatticha pushed a commit to kpatticha/kibana that referenced this pull request Apr 25, 2023
Closes elastic#132457

## Summary

This PR adds a reset button to the dashboard top navigation in both edit
and view mode - when clicked, if the dashboard has unsaved changes, it
will revert the dashboard to the last saved state:



https://user-images.githubusercontent.com/8698078/232918433-97bac4b0-7472-49e9-9eb3-2cb7c9e6edf6.mov

> **Note**
> The above video contains some old copy for the modals. Please refer to
"All copy changes" below to see the updated copy.

Note that, by adding more buttons to the top nav bar, we are increasing
the risk of someone hitting [this accessibility
issue](elastic#154414) (where the
breadcrumbs get completely overlapped before the browser is small enough
for the navigation to collapse) - this will either be addressed on the
Shared UX side or will be addressed as part of our [nav bar
redesign](elastic#154945) 👍


### All Copy Changes

- **Listing Page** 

  | Before | After |
  |--------|-------|
| ![Screenshot 2023-04-18 at 4 29 10
PM](https://user-images.githubusercontent.com/8698078/232919138-7be86e97-ebb4-48a9-b8b1-0b970131aa37.png)
|
![image](https://user-images.githubusercontent.com/8698078/232919166-b7bc7b55-5074-485d-ad0a-2fb695367538.png)
|
|
![image](https://user-images.githubusercontent.com/8698078/232920038-6c8c463e-0c7d-49e7-99c8-86b2ae611844.png)
|
![image](https://user-images.githubusercontent.com/8698078/233412233-a785d99d-9d07-4ee5-a121-646bbd839f7c.png)
|

- **Dashboard - _Edit Mode_** 

  | Before | After |
  |--------|-------|
|
![image](https://user-images.githubusercontent.com/8698078/232920992-2d1b61f4-dff2-4bdd-854b-9cd6fcae07ce.png)
|
![image](https://user-images.githubusercontent.com/8698078/233412732-254a9503-5526-44bc-a2e0-067f8800ad26.png)
|





- **Dashboard - _View Mode_** 

  | Before | After |
  |--------|-------|
| N/A since you couldn't discard changes from view mode previously |
![image](https://user-images.githubusercontent.com/8698078/233413029-9a6b4256-3002-48c5-8620-7596d8f08153.png)
|


###  Flaky Test Runner
- `test/functional/apps/dashboard/group1/dashboard_unsaved_state.ts`
<a
href="https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2144"><img
src="https://user-images.githubusercontent.com/8698078/232622312-8532bc7e-a699-45ee-862d-739d116c5dba.png"></a>
-
`test/functional/apps/dashboard_elements/controls/options_list/options_list_dashboard_interaction.ts`
<a
href="https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2143"><img
src="https://user-images.githubusercontent.com/8698078/232615061-f01439e8-3a69-4190-8b6f-1926e1fa776a.png"></a>


### Checklist

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [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] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
(Refer above)
- [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)
@KOTungseth KOTungseth mentioned this pull request Apr 25, 2023
19 tasks
KOTungseth added a commit that referenced this pull request May 16, 2023
## Summary

Adds the docs for the following 8.8 Presentation docs:

- Unified dashboard settings:
#153862
Docs preview:
https://kibana_157765.docs-preview.app.elstc.co/guide/en/kibana/master/dashboard.html#add-dashboard-settings

- Add reset button: #154872
Docs preview:
https://kibana_157765.docs-preview.app.elstc.co/guide/en/kibana/master/dashboard.html#reset-the-dashboard

---------

Co-authored-by: Nick Peihl <[email protected]>
Co-authored-by: Hannah Mudge <[email protected]>
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request May 16, 2023
## Summary

Adds the docs for the following 8.8 Presentation docs:

- Unified dashboard settings:
elastic#153862
Docs preview:
https://kibana_157765.docs-preview.app.elstc.co/guide/en/kibana/master/dashboard.html#add-dashboard-settings

- Add reset button: elastic#154872
Docs preview:
https://kibana_157765.docs-preview.app.elstc.co/guide/en/kibana/master/dashboard.html#reset-the-dashboard

---------

Co-authored-by: Nick Peihl <[email protected]>
Co-authored-by: Hannah Mudge <[email protected]>
(cherry picked from commit 0689c63)
kibanamachine referenced this pull request May 16, 2023
# Backport

This will backport the following commits from `main` to `8.8`:
- [[DOCS] Adds the presentation 8.8 docs
(#157765)](#157765)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Kaarina
Tungseth","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-05-16T19:21:01Z","message":"[DOCS]
Adds the presentation 8.8 docs (#157765)\n\n## Summary\r\n\r\nAdds the
docs for the following 8.8 Presentation docs:\r\n\r\n- Unified dashboard
settings:\r\nhttps://github.com//pull/153862\r\nDocs
preview:\r\nhttps://kibana_157765.docs-preview.app.elstc.co/guide/en/kibana/master/dashboard.html#add-dashboard-settings\r\n\r\n-
Add reset button: https://github.com/elastic/kibana/pull/154872\r\nDocs
preview:\r\nhttps://kibana_157765.docs-preview.app.elstc.co/guide/en/kibana/master/dashboard.html#reset-the-dashboard\r\n\r\n---------\r\n\r\nCo-authored-by:
Nick Peihl <[email protected]>\r\nCo-authored-by: Hannah Mudge
<[email protected]>","sha":"0689c638d3b3e4c8a9d00e1c2da2412a257771ce","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Docs","Team:Presentation","release_note:skip","v8.8.0","v8.9.0"],"number":157765,"url":"https://github.com/elastic/kibana/pull/157765","mergeCommit":{"message":"[DOCS]
Adds the presentation 8.8 docs (#157765)\n\n## Summary\r\n\r\nAdds the
docs for the following 8.8 Presentation docs:\r\n\r\n- Unified dashboard
settings:\r\nhttps://github.com//pull/153862\r\nDocs
preview:\r\nhttps://kibana_157765.docs-preview.app.elstc.co/guide/en/kibana/master/dashboard.html#add-dashboard-settings\r\n\r\n-
Add reset button: https://github.com/elastic/kibana/pull/154872\r\nDocs
preview:\r\nhttps://kibana_157765.docs-preview.app.elstc.co/guide/en/kibana/master/dashboard.html#reset-the-dashboard\r\n\r\n---------\r\n\r\nCo-authored-by:
Nick Peihl <[email protected]>\r\nCo-authored-by: Hannah Mudge
<[email protected]>","sha":"0689c638d3b3e4c8a9d00e1c2da2412a257771ce"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/157765","number":157765,"mergeCommit":{"message":"[DOCS]
Adds the presentation 8.8 docs (#157765)\n\n## Summary\r\n\r\nAdds the
docs for the following 8.8 Presentation docs:\r\n\r\n- Unified dashboard
settings:\r\nhttps://github.com//pull/153862\r\nDocs
preview:\r\nhttps://kibana_157765.docs-preview.app.elstc.co/guide/en/kibana/master/dashboard.html#add-dashboard-settings\r\n\r\n-
Add reset button: https://github.com/elastic/kibana/pull/154872\r\nDocs
preview:\r\nhttps://kibana_157765.docs-preview.app.elstc.co/guide/en/kibana/master/dashboard.html#reset-the-dashboard\r\n\r\n---------\r\n\r\nCo-authored-by:
Nick Peihl <[email protected]>\r\nCo-authored-by: Hannah Mudge
<[email protected]>","sha":"0689c638d3b3e4c8a9d00e1c2da2412a257771ce"}}]}]
BACKPORT-->

Co-authored-by: Kaarina Tungseth <[email protected]>
jasonrhodes pushed a commit that referenced this pull request May 17, 2023
## Summary

Adds the docs for the following 8.8 Presentation docs:

- Unified dashboard settings:
#153862
Docs preview:
https://kibana_157765.docs-preview.app.elstc.co/guide/en/kibana/master/dashboard.html#add-dashboard-settings

- Add reset button: #154872
Docs preview:
https://kibana_157765.docs-preview.app.elstc.co/guide/en/kibana/master/dashboard.html#reset-the-dashboard

---------

Co-authored-by: Nick Peihl <[email protected]>
Co-authored-by: Hannah Mudge <[email protected]>
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 Feature:Dashboard Dashboard related features Feature:Embedding Embedding content via iFrame impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:medium Medium Level of Effort release_note:enhancement Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Dashboard] Add Reset Function to View Mode
7 participants