[Lens] Add toast notification when visualization is saved#80788
[Lens] Add toast notification when visualization is saved#80788flash1293 merged 5 commits intoelastic:masterfrom
Conversation
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
💚 CLA has been signed |
|
Pinging @elastic/kibana-app (Team:KibanaApp) |
flash1293
left a comment
There was a problem hiding this comment.
Thanks for working on this! Left a small comment in the code.
| isSaveModalVisible: false, | ||
| isLinkedToOriginatingApp: false, | ||
| })); | ||
| notifications.toasts.addSuccess( |
There was a problem hiding this comment.
You put the same notification in this conditional branch and in the main one (https://github.com/elastic/kibana/pull/80788/files#diff-62b4eca6ab93c938023295869419d9b3031fd1c11470d5a3fec89f0012bbf036R451-R458) - it seems like putting just a single invocation in front of the branched code (line 413 of this file) would achieve the same result. Is there any reason I'm missing you didn't do this?
There was a problem hiding this comment.
You're right! I was actually following where isSaveModalVisible: false was to display the toast notification when the save modal is closed. I just tried your suggestion, and it indeed achieves the same result. I also did a manual test and ran node scripts/jest lens to verify the suggested change.
…e invocation of toast notification
|
Jenkins, test this. |
| } | ||
|
|
||
| notifications.toasts.addSuccess( | ||
| i18n.translate('visualize.topNavMenu.saveVisualization.successNotificationText', { |
There was a problem hiding this comment.
@lykims As this is a separate plugin, you need a separate i18n id - I suggest xpack.lens.app.saveVisualization.successNotificationText
You don't need to add translations manually, this is handled in a separate process
There was a problem hiding this comment.
Done! Interesting. I was wondering how the translation was done.
|
@elasticmachine merge upstream |
|
Jenkins, test this |
|
@elasticmachine merge upstream |
flash1293
left a comment
There was a problem hiding this comment.
Tested in Chrome, works as expected, LGTM
Thanks a lot for taking care of this! I'll wait for the CI and merge it in afterwards.
|
Jenkins, test this |
💚 Build SucceededMetrics [docs]async chunks size
History
To update your PR or re-run it, just comment with: |
* master: (64 commits) Rename Security Solution Bug Template (elastic#81187) Update links (elastic#81125) Specify format for date range query (elastic#81025) [Alerting] Improve toast when alert is created (elastic#80327) [UX] Add empty states (elastic#80904) Add TS config for kibana_legacy (elastic#80992) [Telemetry] Add method to enable endpoint security data usage example (elastic#80940) [Alerting] Add scoped cluster client to alerts and actions services (elastic#80794) Fix reactRouterNavigate when used with a string (elastic#80520) [Security Solution] [Detections] Read privileges for dependencies (elastic#80852) [ML] Fixing exclude frequent in advanced wizard (elastic#81121) Fix security solution template label (elastic#80976) [DOCS] Update index management docs (elastic#80893) [APM] Error rate on service list page is not in sync with the value at the transaction page (elastic#80814) skip flaky suite (elastic#81072) [Task Manager] Cleans up legacy plugin structure (elastic#80381) Support unsigned_long fields (elastic#81115) [Form lib] Export internal state instead of raw state (elastic#80842) [Lens] Add toast notification when visualization is saved (elastic#80788) Index pattern edit field formatter API (elastic#78352) ...
|
Thank you for reviewing and merging this, @flash1293 ! |
Summary
Resolves #59406
When a Lens visualization is saved, there should be a toast notification that indicates that the visualization has been saved successfully. This behaviour is present in other basic visualizations.
Checklist
Delete any items that are not applicable to this PR.
For maintainers