[Graph] Save modal #44261

flash1293 · 2019-08-28T14:33:08Z

Summary

This PR removes the current inline-save menu from Graph and switches to a save modal as used in Visualize and Discover.

Old:

New:

Depends on #44068

Platform team

This also includes TS-ifying show_saved_object_save_modal.tsx

App arch team

This PR slightly extends the saved object save modal to have different labels for the action button for ordinary saving ("Save") or for saving an object with the same name as another already existing object ("Confirm save"). Additionally a small bug was fixed that caused the wrong button label to show in the duplicate title warning message if a custom label is used.

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

elasticmachine · 2019-08-28T15:47:53Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

spalger · 2019-08-28T18:59:32Z

retest

elasticmachine · 2019-08-28T20:14:56Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-08-29T10:00:52Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-08-29T15:20:16Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-04T12:25:11Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

joshdover

Platform changes LGTM

src/legacy/ui/public/saved_objects/show_saved_object_save_modal.tsx

elasticmachine · 2019-09-05T09:01:44Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-05T10:03:49Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

lizozom

I added a couple of comments.

While testing, I noticed that when the popup enters the duplicate name mode, it changes size. This makes it look like an entirely different popup. However, when you go and change the name - the popup is resized again, back to the smaller size.

I would like to suggest enforcing the popup size, or at least the popup width. I think it should improve UX.

src/legacy/ui/public/saved_objects/show_saved_object_save_modal.tsx

src/plugins/kibana_react/public/saved_objects/saved_object_save_modal.tsx

x-pack/legacy/plugins/graph/public/components/graph_save_modal.tsx

flash1293 · 2019-09-05T13:17:34Z

While testing, I noticed that when the popup enters the duplicate name mode, it changes size. This makes it look like an entirely different popup. However, when you go and change the name - the popup is resized again, back to the smaller size.

I would like to suggest enforcing the popup size, or at least the popup width. I think it should improve UX.

I had that in first, but reverted because it's how the modals work in current master everywhere:

Before clicking save:

After clicking save:

I can put it back in if you want.

lizozom · 2019-09-05T13:34:06Z

@flash1293 I would add it back and let @elastic/kibana-design decide. :)

flash1293 · 2019-09-05T13:42:41Z

Put in a fixed with for all save modals, this is how they look in various apps:

Maps:

Dashboard:

Graph:

lizozom

LGTM once green

elasticmachine · 2019-09-05T15:09:50Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

cchaos · 2019-09-05T14:47:53Z

src/plugins/kibana_react/public/saved_objects/saved_object_save_modal.tsx


    return (
      <EuiOverlayMask>
        <form onSubmit={this.onFormSubmit}>
          <EuiModal
            data-test-subj="savedObjectSaveModal"
-            className="dshSaveModal"
+            className="saveModal"


This className needs a prefix to be less specific and match BEM naming conventions. Since this is a Kibana global component I'd call it

Suggested change

className="saveModal"

className="kbnSavedObjectSaveModal"

Good suggestion, I renamed it

cchaos

Thx!

elasticmachine · 2019-09-05T16:36:04Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

* create graph listing page * clean up app folder * remove inline loading menu * also add badge to workspace route * fix tests * fix graph spaces functional test * generate documentation for new breadcrumb property * fix test subject names * remove unused translations * start implementing save modal flow for Graph * fix spaces functional test * wip save modal * wip save modal * add and style save modal * add placeholder to description field * disable dirty check on breadcrumb navigation and fix delete function * improve onClick typing on breadcrumb * fix newline error and use new types in dashboard app controller * fix translation errors * fix i18n translation for real * code review * fix i18n phrases * remove fragments * remove unnecessary max-width and add commentary * move to async syntax * clean up implementation * use description instead of title * fix snapshot * adress review comments and set width for all save modals * fix bug and improve typing * fix classname

…ete-for-distance_feature * 'master' of github.com:elastic/kibana: (89 commits) Replace TSVB timeseries charts with elastic-charts (elastic#33558) [TSVB][Top N aggregation] Unable to deal with negative values (elastic#43581) [alerting] Adds Action Type configuration support and whitelisting (elastic#44483) FTR: fix WebDriver Actions calls (elastic#44605) [Code] add NodeRepositoriesService to watch new repositories on local node (elastic#44677) [skip-ci][Maps] Improve Maps intro page (elastic#44721) [Maps] Update titles and descriptions for data sources (elastic#44833) Types + Extract Integration Util (elastic#44433) Downgrade log level from info to debug for cases when we cannot handle authentication attempt. (elastic#44933) [Reporting] Remove Chome stdout/stderr observables, Add Browser Logger observable (elastic#44359) Update Jest script to output coverage (elastic#44447) [ftr] support --kibana-install-dir flag (elastic#44552) [WATCHER] Allow user to set a threshold value of 0 (elastic#44810) Remove injectI18n in dashboard plugin. (elastic#44580) [Graph] Save modal (elastic#44261) Use external script for the OIDC Implicit flow handler page. (elastic#44866) disable router prefixing with pluginId (elastic#44855) [SIEM] Fix bug on url + inspect functionality on hosts/hostDetails page (elastic#44671) [ML] File data viz limiting uploaded doc chunk size (elastic#44768) [code] Append go env variable 'GOCACHE' to go lsp spawn command. (elastic#44864) ...

…plate * 'master' of github.com:elastic/kibana: (91 commits) [APM] Make number of x ticks responsive to the plot width (elastic#44870) [ML] Single metric viewer: Fix top nav refresh behaviour. (elastic#44860) Replace TSVB timeseries charts with elastic-charts (elastic#33558) [TSVB][Top N aggregation] Unable to deal with negative values (elastic#43581) [alerting] Adds Action Type configuration support and whitelisting (elastic#44483) FTR: fix WebDriver Actions calls (elastic#44605) [Code] add NodeRepositoriesService to watch new repositories on local node (elastic#44677) [skip-ci][Maps] Improve Maps intro page (elastic#44721) [Maps] Update titles and descriptions for data sources (elastic#44833) Types + Extract Integration Util (elastic#44433) Downgrade log level from info to debug for cases when we cannot handle authentication attempt. (elastic#44933) [Reporting] Remove Chome stdout/stderr observables, Add Browser Logger observable (elastic#44359) Update Jest script to output coverage (elastic#44447) [ftr] support --kibana-install-dir flag (elastic#44552) [WATCHER] Allow user to set a threshold value of 0 (elastic#44810) Remove injectI18n in dashboard plugin. (elastic#44580) [Graph] Save modal (elastic#44261) Use external script for the OIDC Implicit flow handler page. (elastic#44866) disable router prefixing with pluginId (elastic#44855) [SIEM] Fix bug on url + inspect functionality on hosts/hostDetails page (elastic#44671) ...

flash1293 added 14 commits August 26, 2019 17:56

create graph listing page

067158a

clean up app folder

5fddc10

remove inline loading menu

95f003b

also add badge to workspace route

ae44388

fix tests

2f1056f

fix graph spaces functional test

5d637f5

generate documentation for new breadcrumb property

9d17862

fix test subject names

db1d740

remove unused translations

e09b70d

start implementing save modal flow for Graph

2dab994

fix spaces functional test

c34d845

wip save modal

2d775e6

wip save modal

ebe80d1

add and style save modal

fab1577

flash1293 added the Feature:Graph Graph application feature label Aug 28, 2019

flash1293 added 2 commits August 28, 2019 16:44

add placeholder to description field

a9d31c8

Merge remote-tracking branch 'upstream/master' into graph/listing-page

9d2822e

flash1293 added 3 commits August 29, 2019 10:03

Merge remote-tracking branch 'upstream/master' into graph/listing-page

d0b9247

disable dirty check on breadcrumb navigation and fix delete function

d187eae

Merge branch 'graph/listing-page' into graph/savemodal

21ba056

flash1293 added 3 commits August 29, 2019 15:36

improve onClick typing on breadcrumb

a289d7d

Merge branch 'graph/listing-page' into graph/savemodal

989481c

fix newline error and use new types in dashboard app controller

be1e59a

flash1293 changed the title ~~[Graph] Implement save modal~~ [Graph] Save modal Aug 29, 2019

fix translation errors

c5ac49b

joshdover approved these changes Sep 4, 2019

View reviewed changes

src/legacy/ui/public/saved_objects/show_saved_object_save_modal.tsx Outdated Show resolved Hide resolved

src/legacy/ui/public/saved_objects/show_saved_object_save_modal.tsx Outdated Show resolved Hide resolved

flash1293 added 5 commits September 5, 2019 09:34

remove unnecessary max-width and add commentary

bcc280f

move to async syntax

a578814

clean up implementation

4897b52

use description instead of title

7b2b91b

Merge remote-tracking branch 'upstream/master' into graph/savemodal

208b4db

flash1293 added the v7.5.0 label Sep 5, 2019

fix snapshot

e4b9aa0

lizozom reviewed Sep 5, 2019

View reviewed changes

adress review comments and set width for all save modals

1195f36

lizozom approved these changes Sep 5, 2019

View reviewed changes

fix bug and improve typing

372f3b4

cchaos suggested changes Sep 5, 2019

View reviewed changes

fix classname

3de7e4f

cchaos approved these changes Sep 5, 2019

View reviewed changes

flash1293 merged commit f080218 into elastic:master Sep 5, 2019

flash1293 deleted the graph/savemodal branch September 5, 2019 17:39

flash1293 mentioned this pull request Sep 5, 2019

[7.x] [Graph] Save modal (#44261) #44918

Merged

flash1293 added a commit that referenced this pull request Sep 6, 2019

[Graph] Save modal (#44261) (#44918)

cffc08a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Graph] Save modal #44261

[Graph] Save modal #44261

flash1293 commented Aug 28, 2019 •

edited

Loading

elasticmachine commented Aug 28, 2019

spalger commented Aug 28, 2019

elasticmachine commented Aug 28, 2019

elasticmachine commented Aug 29, 2019

elasticmachine commented Aug 29, 2019

elasticmachine commented Sep 4, 2019

joshdover left a comment

elasticmachine commented Sep 5, 2019

elasticmachine commented Sep 5, 2019

lizozom left a comment

flash1293 commented Sep 5, 2019

lizozom commented Sep 5, 2019

flash1293 commented Sep 5, 2019

lizozom left a comment

elasticmachine commented Sep 5, 2019

cchaos Sep 5, 2019

flash1293 Sep 5, 2019

cchaos left a comment

elasticmachine commented Sep 5, 2019

[Graph] Save modal #44261

[Graph] Save modal #44261

Conversation

flash1293 commented Aug 28, 2019 • edited Loading

Summary

Platform team

App arch team

Checklist

For maintainers

elasticmachine commented Aug 28, 2019

💔 Build Failed

spalger commented Aug 28, 2019

elasticmachine commented Aug 28, 2019

💔 Build Failed

elasticmachine commented Aug 29, 2019

💔 Build Failed

elasticmachine commented Aug 29, 2019

💔 Build Failed

elasticmachine commented Sep 4, 2019

💚 Build Succeeded

joshdover left a comment

Choose a reason for hiding this comment

elasticmachine commented Sep 5, 2019

💔 Build Failed

elasticmachine commented Sep 5, 2019

💚 Build Succeeded

lizozom left a comment

Choose a reason for hiding this comment

flash1293 commented Sep 5, 2019

lizozom commented Sep 5, 2019

flash1293 commented Sep 5, 2019

lizozom left a comment

Choose a reason for hiding this comment

elasticmachine commented Sep 5, 2019

💔 Build Failed

cchaos Sep 5, 2019

Choose a reason for hiding this comment

flash1293 Sep 5, 2019

Choose a reason for hiding this comment

cchaos left a comment

Choose a reason for hiding this comment

elasticmachine commented Sep 5, 2019

💚 Build Succeeded

flash1293 commented Aug 28, 2019 •

edited

Loading