Reactify visualize app #67848

sulemanof · 2020-06-01T12:22:34Z

Summary

De-Angularize Visualize app

Closes #59811
Closes #65613
Closes #68509 , can't reproduce it with these changes.

The main aim of the PR is to get rid of Angular and use React & React-Router in Visualize.
Visualize still uses hash router internally; moving it to browser router which uses the ScopedHistory object under the hood is a separate task, since it will cause a lot of changes in other places.

Visual changes

I've mostly tried to save existing behavior, but eventually added a few changes.
Here they are:

experimental vis info banner was changed to use EuiCallOut:
navigation to nonexistent route now is redirected to the visualize landing page (instead of using kibanaLegacy.navigateToDefaultApp() ) and is accompanied by a banner message:

creating a new vis with invalid params now handled with toasts and redirects to the landing page

Other changes(enhancements)

Vis is not reloaded after save, just URL is replaced;
added additional error handling with toast notifications (e.x. failed to create a visualization with invalid type or without an index pattern).

Note: Unit tests will be provided in a separate PR to decouple reviewers work 🙂

Checklist

Delete any items that are not applicable to this PR.

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
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

src/plugins/visualize/public/application/types.ts

src/plugins/visualize/public/plugin.ts

sulemanof · 2020-06-12T11:13:26Z

src/plugins/visualize/public/application/utils/index.ts

@@ -17,12 +17,8 @@
 * under the License.
 */

-import { i18n } from '@kbn/i18n';
-
-export const defaultFeedbackMessage = i18n.translate('kibana_utils.defaultFeedbackMessage', {


Instead of exporting defaultFeedbackMessage from kibana_utils to every experimental vis (input_control, vega, metrics), I moved it into visualize: https://github.com/elastic/kibana/pull/67848/files#diff-1d6c318a6f1b7dcab619931a74884297

sulemanof · 2020-06-12T11:17:29Z

src/plugins/vis_default_editor/public/components/sidebar/sidebar.tsx

@@ -104,12 +110,12 @@ function DefaultEditorSideBar({
        aggs: state.data.aggs ? (state.data.aggs.aggs.map((agg) => agg.toJSON()) as any) : [],
      },
    });
-    eventEmitter.emit('updateVis');
+    embeddableHandler.reload();


Instead of emitting an event from a particular editor, and handle it in visualize as:

reload it inside an editor outright (IMHO it's more evident)

flash1293 · 2020-06-15T14:51:59Z

@elasticmachine merge upstream

elasticmachine · 2020-06-15T14:52:01Z

merge conflict between base and head

flash1293

Not finished yet, will continue tomorrow

flash1293 · 2020-06-15T14:48:16Z

src/plugins/visualizations/public/saved_visualizations/saved_visualizations.ts

@@ -56,7 +56,7 @@ export function createSavedVisLoader(services: SavedObjectKibanaServicesWithVisu
      source.icon = source.type.icon;
      source.image = source.type.image;
      source.typeTitle = source.type.title;
-      source.editUrl = `#/edit/${id}`;
+      source.editUrl = `/edit/${id}`;


Seems like this should be #/edit/${id} for now as the PR isn't switching to path routing yet.

If I'm not wrong, this is used only used in visualize for navigating to a visualization from the listing page. In new implementation, history.push is used for navigating, where the history object is a hash history object, so it accepts paths without leading #.
Apart from this case, I didn't find any usages of this.
Navigating from other apps (e.x. from discover/dashboard/canvas) works well.
If I missed smth, please let me know! 🙂

flash1293 · 2020-06-15T14:50:56Z

src/plugins/visualize/public/application/app.tsx

+      >
+        <VisualizeListing />
+      </Route>
+      <Redirect to={VisualizeConstants.LANDING_PAGE_PATH} />


Side note: #68284 will run into conflicts with this because it changes the redirect logic on unmatched routes. We can solve that on the other PR though, no strong opinion on what gets merged first. #68284 is a blocker for 7.9 but otherwise it should be fine.

Replaced with the VisualizeNoMatch component

src/plugins/visualize/public/application/utils/use/use_chrome.ts

flash1293

Not sure whether introduced by this change or something else, but the search bar is shown in embedded mode:

src/plugins/visualize/public/application/components/visualize_listing.tsx

flash1293

UI state is not persisted:

Create area chart with multiple series
Change color of one series by clicking the dot in the legend and selecting another color
Save vis
Go to listing page
Select vis again
Color is not selected anymore

Same for table sorting in TSVB

# Conflicts: # src/plugins/visualize/public/application/legacy_app.js

sulemanof · 2020-06-17T11:50:11Z

Not sure whether introduced by this change or something else, but the search bar is shown in embedded mode:

Hm, interesting things happen around embeded mode.
In kibana 7.4.0, 7.5.0 full navigation panel exists:

but it doesn't exist in 7.0.0 up to 7.4.0, and from 7.6.0 up to master.
So what is the expected behavior?

flash1293 · 2020-06-17T12:00:35Z

Good question. @timroes do you know what's up with that?

stratoula

Code seems LGTM to me, but I haven't tested it yet. Will continue it asap. I can confirm so far that #68509 is fixed 🍰

src/plugins/visualize/public/application/components/visualize_editor.tsx

src/plugins/visualize/public/application/utils/get_table_columns.tsx

…lize

alexwizp

LGTM

stratoula

Tested on Chrome, seems LGTM

sulemanof · 2020-06-23T08:42:13Z

@elasticmachine merge upstream

cchaos

SASS files LGTM

# Conflicts: # src/plugins/visualize/public/application/editor/editor.js # src/plugins/visualize/public/application/legacy_app.js # src/plugins/visualize/public/kibana_services.ts # src/plugins/visualize/public/plugin.ts

flash1293

Only tested the legacy redirect logic in Chrome and Firefox, works like a charm. LGTM 👍

It even fixes the back button breakage on paths that are normalized on the fly 💯

@majagrubic @ThomThomson This PR is a great blueprint for doing the same thing in Dashboard

streamich

AppArch code changes LGTM.

…lize # Conflicts: # src/plugins/visualize/public/application/editor/editor.js

…nto np/reactify_visualize

kibanamachine · 2020-06-29T14:20:45Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: c1e999a

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
kibanaUtils	158	-1	159
visualize	179	+4	175
total	-	+3	-

History

💚 Build #56211 succeeded 60c9f16
💚 Build #55462 succeeded 160dbb9
💚 Build #55317 succeeded 03dec22
💚 Build #54944 succeeded 6d51a08
💔 Build #54841 failed a43752a

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

* Reactify visualize app * Fix typescript failures after merging master * Make sure refresh button works * Subscribe filter manager fetches * Use redirect to landing page * Update savedSearch type * Add check for TSVB is loaded * Fix comments * Fix uiState persistence on vis load * Remove extra div around TableListView * Update DTS selectors * Add error handling for embeddable * Remove extra argument from useEditorUpdates effect * Update comments, fix typos * Remove extra div wrapper * Apply design suggestions * Revert accidental config changes * Apply navigating to dashboard * Apply redirect legacy urls * Apply incoming changes * Apply incoming changes Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* Reactify visualize app * Fix typescript failures after merging master * Make sure refresh button works * Subscribe filter manager fetches * Use redirect to landing page * Update savedSearch type * Add check for TSVB is loaded * Fix comments * Fix uiState persistence on vis load * Remove extra div around TableListView * Update DTS selectors * Add error handling for embeddable * Remove extra argument from useEditorUpdates effect * Update comments, fix typos * Remove extra div wrapper * Apply design suggestions * Revert accidental config changes * Apply navigating to dashboard * Apply redirect legacy urls * Apply incoming changes * Apply incoming changes Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

sulemanof added v7.9.0 v8.0.0 labels Jun 1, 2020

sulemanof mentioned this pull request Jun 9, 2020

[TSVB] Loading TSVB in editor initially sends wrong timeRange #68509

Closed

sulemanof requested review from flash1293, stratoula, DianaDerevyankina, VladLasitsa and alexwizp June 10, 2020 11:46

alexwizp reviewed Jun 11, 2020

View reviewed changes

src/plugins/visualize/public/application/types.ts Outdated Show resolved Hide resolved

alexwizp reviewed Jun 11, 2020

View reviewed changes

src/plugins/visualize/public/plugin.ts Show resolved Hide resolved

sulemanof force-pushed the np/reactify_visualize branch from 11eca48 to ba5ac21 Compare June 12, 2020 07:58

sulemanof added 6 commits June 12, 2020 10:58

Reactify visualize app

ba5ac21

Fix typescript failures after merging master

727583a

Make sure refresh button works

fcc3508

Subscribe filter manager fetches

f2691e7

Use redirect to landing page

619e408

Update savedSearch type

e5f45a6

sulemanof commented Jun 12, 2020

View reviewed changes

Add check for TSVB is loaded

ef776c1

flash1293 reviewed Jun 15, 2020

View reviewed changes

flash1293 reviewed Jun 16, 2020

View reviewed changes

src/plugins/visualize/public/application/components/visualize_listing.tsx Outdated Show resolved Hide resolved

flash1293 reviewed Jun 16, 2020

View reviewed changes

Merge branch 'master' into np/reactify_visualize

3664bf3

# Conflicts: # src/plugins/visualize/public/application/legacy_app.js

Fix comments

cd21384

stratoula reviewed Jun 18, 2020

View reviewed changes

cchaos reviewed Jun 18, 2020

View reviewed changes

src/plugins/visualize/public/application/components/visualize_editor.tsx Outdated Show resolved Hide resolved

src/plugins/visualize/public/application/utils/get_table_columns.tsx Outdated Show resolved Hide resolved

sulemanof added 5 commits June 19, 2020 17:27

Remove extra argument from useEditorUpdates effect

cc84a37

Update comments, fix typos

5ff42f4

Remove extra div wrapper

fc15832

Apply design suggestions

e90e0b6

Merge remote-tracking branch 'upstream/master' into np/reactify_visua…

3566a34

…lize

sulemanof requested a review from a team as a code owner June 19, 2020 15:08

Revert accidental config changes

03dec22

alexwizp approved these changes Jun 23, 2020

View reviewed changes

stratoula approved these changes Jun 23, 2020

View reviewed changes

Merge branch 'master' into np/reactify_visualize

160dbb9

cchaos approved these changes Jun 23, 2020

View reviewed changes

sulemanof added 3 commits June 24, 2020 17:10

Merge branch 'master' into np/reactify_visualize

ca17287

# Conflicts: # src/plugins/visualize/public/application/editor/editor.js # src/plugins/visualize/public/application/legacy_app.js # src/plugins/visualize/public/kibana_services.ts # src/plugins/visualize/public/plugin.ts

Apply navigating to dashboard

3e943e2

Apply redirect legacy urls

60c9f16

flash1293 approved these changes Jun 25, 2020

View reviewed changes

streamich approved these changes Jun 25, 2020

View reviewed changes

sulemanof added 4 commits June 29, 2020 12:06

Merge remote-tracking branch 'upstream/master' into np/reactify_visua…

c9572f5

…lize # Conflicts: # src/plugins/visualize/public/application/editor/editor.js

Apply incoming changes

151e6a1

Apply incoming changes

1120db1

Merge branch 'np/reactify_visualize' of github.com:sulemanof/kibana i…

c1e999a

…nto np/reactify_visualize

sulemanof merged commit 19bda1f into elastic:master Jun 29, 2020

sulemanof deleted the np/reactify_visualize branch June 29, 2020 14:21

sulemanof mentioned this pull request Jun 29, 2020

[7.x] Reactify visualize app (#67848) #70205

Merged

sulemanof mentioned this pull request Dec 2, 2020

Browser back/forward does not update filtered data #84711

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reactify visualize app #67848

Reactify visualize app #67848

sulemanof commented Jun 1, 2020 •

edited

Loading

sulemanof Jun 12, 2020

sulemanof Jun 12, 2020

flash1293 commented Jun 15, 2020

elasticmachine commented Jun 15, 2020

flash1293 left a comment

flash1293 Jun 15, 2020

sulemanof Jun 17, 2020

flash1293 Jun 15, 2020

sulemanof Jun 25, 2020

flash1293 left a comment

flash1293 left a comment

sulemanof commented Jun 17, 2020

flash1293 commented Jun 17, 2020

stratoula left a comment

alexwizp left a comment

stratoula left a comment

sulemanof commented Jun 23, 2020

cchaos left a comment

flash1293 left a comment •

edited

Loading

streamich left a comment

kibanamachine commented Jun 29, 2020

Reactify visualize app #67848

Reactify visualize app #67848

Conversation

sulemanof commented Jun 1, 2020 • edited Loading

Summary

De-Angularize Visualize app

Visual changes

Other changes(enhancements)

Note: Unit tests will be provided in a separate PR to decouple reviewers work 🙂

Checklist

For maintainers

sulemanof Jun 12, 2020

Choose a reason for hiding this comment

sulemanof Jun 12, 2020

Choose a reason for hiding this comment

flash1293 commented Jun 15, 2020

elasticmachine commented Jun 15, 2020

flash1293 left a comment

Choose a reason for hiding this comment

flash1293 Jun 15, 2020

Choose a reason for hiding this comment

sulemanof Jun 17, 2020

Choose a reason for hiding this comment

flash1293 Jun 15, 2020

Choose a reason for hiding this comment

sulemanof Jun 25, 2020

Choose a reason for hiding this comment

flash1293 left a comment

Choose a reason for hiding this comment

flash1293 left a comment

Choose a reason for hiding this comment

sulemanof commented Jun 17, 2020

flash1293 commented Jun 17, 2020

stratoula left a comment

Choose a reason for hiding this comment

alexwizp left a comment

Choose a reason for hiding this comment

stratoula left a comment

Choose a reason for hiding this comment

sulemanof commented Jun 23, 2020

cchaos left a comment

Choose a reason for hiding this comment

flash1293 left a comment • edited Loading

Choose a reason for hiding this comment

streamich left a comment

Choose a reason for hiding this comment

kibanamachine commented Jun 29, 2020

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

History

sulemanof commented Jun 1, 2020 •

edited

Loading

flash1293 left a comment •

edited

Loading