[Graph] Deangularize graph app controller #106587

dimaanj · 2021-07-22T18:14:57Z

Part of #91128

Summary

Graph app state management is migrated to react, namely steps 2-7 of graph deangularization guideline is implemented
UI remains the same after this changes

Implementation notes

The main mutable workspace object stored in the react ref
After each action on worksapce, react render should be forced. It's done via updating renderCounter and providing it to components which needs update.
asSyncedObservable still needed, since some of the workspace internals should be provided to mounted <Settings /> overlay. It should be removed after migrating thouse worksapce stuff to redux

Testing notes

Field actions (select/deselect, disable, change settings)
Control panel actions
TopNavMenu options: new, save, inspect panel and settings (advanced settings, block list, drill downs)
Graph nodes/edges selection actions
Open a new/existing workspace via URL with/without query param
Readonly graph

dimaanj · 2021-07-26T08:43:07Z

@elasticmachine merge upstream

…e-angular-from-control-panel # Conflicts: # x-pack/plugins/graph/public/app.js

dimaanj · 2021-07-30T18:03:20Z

@elasticmachine merge upstream

…e-angular-from-control-panel

…e-angular-from-control-panel # Conflicts: # x-pack/plugins/graph/public/types/workspace_state.ts

dimaanj · 2021-08-15T19:09:30Z

@elasticmachine merge upstream

dimaanj · 2021-08-30T08:46:06Z

@elasticmachine merge upstream

dej611

Clicking on New workspace now leads to the Graph main index page rather than creating a new workspace ❌ - blocker?

Can still reproduce this one.

There are also some design regressions:

when configuring the initial data source for the graph, the scroll bar appears now (only in Safari). FF and Chrome are ok. (probably blocker?)
The "quick selection"s button pills are more "compact" compared to the previous implementation (no blocker)

x-pack/plugins/graph/public/components/control_panel/control_panel.tsx

x-pack/plugins/graph/public/helpers/saved_workspace_utils.ts

x-pack/plugins/graph/public/components/settings/settings.test.tsx

dej611

Left few minor nitpick comments, but generally ok with the content. 👍

x-pack/plugins/graph/public/plugin.ts

kertal · 2021-08-31T10:58:04Z

@elasticmachine merge upstream

kibanamachine · 2021-08-31T13:15:42Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`graph`	157	167	+10

Async chunks

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

id	before	after	diff
`graph`	1.1MB	656.8KB	-500.9KB

Page load bundle

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

id	before	after	diff
`graph`	11.5KB	11.1KB	-367.0B

Unknown metric groups

References to deprecated APIs

id	before	after	diff
`graph`	65	97	+32

History

💔 Build #149688 failed 3e6bac2
💚 Build #149392 succeeded 7ed9068
💔 Build #149357 failed 0da7071
💔 Build #149281 failed 271ed75
💚 Build #149248 succeeded a002fcf

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

cc @dmitriynj

kertal

!!!

Did another quick check in Firefox, Chrome, Safari, everything works as expected. Great work! 👍

ryankeairns · 2021-08-31T14:42:17Z

x-pack/plugins/graph/public/_main.scss

@@ -21,6 +21,7 @@
 */

 .gphNoUserSelect {
+  padding-right: $euiSizeXS;


I see this adds a bit of padding to the
node labels, but it's not clear what problem this addresses. Out of curiosity, was there a particular situation that led to this addition?

I guess it was mistakenly leaved and can be removed in a follow up PR.

ryankeairns

Minor CSS considerations.
We could do a larger style pass in the future.

ryankeairns · 2021-08-31T14:45:42Z

x-pack/plugins/graph/public/components/_sidebar.scss

@@ -24,6 +24,10 @@
  padding: $euiSizeXS;
  border-radius: $euiBorderRadius;
  margin-bottom: $euiSizeXS;
+
+  & > span {


As a general practice, we tend to not target elements directly using a plain element selector. Instead, we prefer to target a class which in this case seems possible via .kuiIcon (class on the span).

Thank you, let's fix it in follow up PR.

* [Graph] deaungularize control panel * [Graph] move main graph directive to react * [Graph] refactoring * [Graph] remove redundant memoization, update import * [Graph] fix settings menu, clean up the code * [Graph] fix graph settings * [Graph] code refactoring, fixing control panel render issues * [Graph] fix small mistake * [Graph] rename components * [Graph] fix imports * [Graph] fix graph search and inspect panel * [Graph] remove redundant types * [Graph] fix problem with selection list * [Graph] fix functional test which uses selection list * [Graph] fix unit tests, update types * [Graph] fix types * [Discover] fix url queries * [Graph] fix types * [Graph] add react router, remove angular stuff * [Graph] fix styles * [Graph] fix i18n * [Graph] fix navigation to a new workspace creation * [Graph] fix issues from comments * [Graph] add suggested changed * Update x-pack/plugins/graph/public/components/graph_visualization/graph_visualization.tsx Co-authored-by: Marco Liberati <[email protected]> * [Graph] remove brace lib from imports * [Graph] fix url navigation between workspaces, fix types * [Graph] refactoring, fixing url issue * [Graph] update graph dependencies * [Graph] add comments * [Graph] fix types * [Graph] fix new button, fix control panel styles * [Graph] apply suggestions Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Marco Liberati <[email protected]>

* [Graph] deaungularize control panel * [Graph] move main graph directive to react * [Graph] refactoring * [Graph] remove redundant memoization, update import * [Graph] fix settings menu, clean up the code * [Graph] fix graph settings * [Graph] code refactoring, fixing control panel render issues * [Graph] fix small mistake * [Graph] rename components * [Graph] fix imports * [Graph] fix graph search and inspect panel * [Graph] remove redundant types * [Graph] fix problem with selection list * [Graph] fix functional test which uses selection list * [Graph] fix unit tests, update types * [Graph] fix types * [Discover] fix url queries * [Graph] fix types * [Graph] add react router, remove angular stuff * [Graph] fix styles * [Graph] fix i18n * [Graph] fix navigation to a new workspace creation * [Graph] fix issues from comments * [Graph] add suggested changed * Update x-pack/plugins/graph/public/components/graph_visualization/graph_visualization.tsx Co-authored-by: Marco Liberati <[email protected]> * [Graph] remove brace lib from imports * [Graph] fix url navigation between workspaces, fix types * [Graph] refactoring, fixing url issue * [Graph] update graph dependencies * [Graph] add comments * [Graph] fix types * [Graph] fix new button, fix control panel styles * [Graph] apply suggestions Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Marco Liberati <[email protected]> Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Marco Liberati <[email protected]>

[Graph] deaungularize control panel

ad2a44d

dimaanj added Feature:Graph Graph application feature Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.15.0 labels Jul 22, 2021

dimaanj requested review from timroes and majagrubic July 22, 2021 18:14

dimaanj self-assigned this Jul 22, 2021

dimaanj requested a review from kertal July 26, 2021 07:58

Merge branch 'master' into remove-angular-from-control-panel

ea2356c

dimaanj changed the title ~~[Graph] Deangularize control panel~~ [Graph] Deangularize control panel & main controller Jul 26, 2021

dimaanj added 2 commits July 26, 2021 16:52

Merge branch 'master' of https://github.com/elastic/kibana into remov…

22b3ec8

…e-angular-from-control-panel # Conflicts: # x-pack/plugins/graph/public/app.js

[Graph] move main graph directive to react

6e169e7

kibanamachine and others added 13 commits July 30, 2021 14:03

Merge branch 'master' into remove-angular-from-control-panel

5bb2f8f

Merge branch 'master' of https://github.com/elastic/kibana into remov…

80e5e20

…e-angular-from-control-panel

[Graph] refactoring

d1e8b7e

[Graph] remove redundant memoization, update import

4783be2

[Graph] fix settings menu, clean up the code

e9936c7

Merge branch 'master' of https://github.com/elastic/kibana into remov…

2a6661b

…e-angular-from-control-panel # Conflicts: # x-pack/plugins/graph/public/types/workspace_state.ts

[Graph] fix graph settings

e9af2a5

[Graph] code refactoring, fixing control panel render issues

defe0e5

[Graph] fix small mistake

b6f8980

[Graph] rename components

26b108a

[Graph] fix imports

1a3fdde

[Graph] fix graph search and inspect panel

a345ca6

[Graph] remove redundant types

4ceb34b

kibanamachine and others added 5 commits August 27, 2021 13:58

Merge branch 'master' into remove-angular-from-control-panel

a002fcf

[Graph] refactoring, fixing url issue

a7a930a

[Graph] update graph dependencies

547dbd2

[Graph] add comments

271ed75

[Graph] fix types

d65e132

Merge branch 'master' into remove-angular-from-control-panel

0da7071

dej611 requested changes Aug 30, 2021

View reviewed changes

[Graph] fix new button, fix control panel styles

7ed9068

dimaanj requested a review from dej611 August 30, 2021 11:17

dej611 reviewed Aug 30, 2021

View reviewed changes

x-pack/plugins/graph/public/components/control_panel/control_panel.tsx Outdated Show resolved Hide resolved

dej611 reviewed Aug 30, 2021

View reviewed changes

x-pack/plugins/graph/public/helpers/saved_workspace_utils.ts Outdated Show resolved Hide resolved

dej611 reviewed Aug 30, 2021

View reviewed changes

x-pack/plugins/graph/public/components/settings/settings.test.tsx Outdated Show resolved Hide resolved

dej611 approved these changes Aug 30, 2021

View reviewed changes

kertal reviewed Aug 30, 2021

View reviewed changes

x-pack/plugins/graph/public/plugin.ts Show resolved Hide resolved

[Graph] apply suggestions

3e6bac2

Merge branch 'master' into remove-angular-from-control-panel

fbd54b4

kertal self-requested a review August 31, 2021 14:09

kertal approved these changes Aug 31, 2021

View reviewed changes

ryankeairns reviewed Aug 31, 2021

View reviewed changes

ryankeairns approved these changes Aug 31, 2021

View reviewed changes

dimaanj merged commit 3f7c461 into elastic:master Aug 31, 2021

dimaanj mentioned this pull request Aug 31, 2021

[7.x] [Graph] Deangularize graph app controller (#106587) #110669

Merged

kertal mentioned this pull request Sep 2, 2021

[Graph] De-angularization of the app #91128

Closed

mbondyra mentioned this pull request Sep 10, 2021

[Graph] WorkspaceRoute and WorkspaceLayout rerender 308 times on start #111861

Closed

jughosta mentioned this pull request Sep 20, 2022

[graph] Selected index name is always missing in request inspector #72530

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Graph] Deangularize graph app controller #106587

[Graph] Deangularize graph app controller #106587

dimaanj commented Jul 22, 2021 •

edited

Loading

dimaanj commented Jul 26, 2021

dimaanj commented Jul 30, 2021

dimaanj commented Aug 15, 2021

dimaanj commented Aug 30, 2021

dej611 left a comment

dej611 left a comment

kertal commented Aug 31, 2021

kibanamachine commented Aug 31, 2021

References to deprecated APIs

kertal left a comment

ryankeairns Aug 31, 2021

dimaanj Aug 31, 2021

ryankeairns left a comment

ryankeairns Aug 31, 2021

dimaanj Aug 31, 2021

[Graph] Deangularize graph app controller #106587

[Graph] Deangularize graph app controller #106587

Conversation

dimaanj commented Jul 22, 2021 • edited Loading

Summary

Implementation notes

Testing notes

dimaanj commented Jul 26, 2021

dimaanj commented Jul 30, 2021

dimaanj commented Aug 15, 2021

dimaanj commented Aug 30, 2021

dej611 left a comment

Choose a reason for hiding this comment

dej611 left a comment

Choose a reason for hiding this comment

kertal commented Aug 31, 2021

kibanamachine commented Aug 31, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

References to deprecated APIs

History

kertal left a comment

Choose a reason for hiding this comment

ryankeairns Aug 31, 2021

Choose a reason for hiding this comment

dimaanj Aug 31, 2021

Choose a reason for hiding this comment

ryankeairns left a comment

Choose a reason for hiding this comment

ryankeairns Aug 31, 2021

Choose a reason for hiding this comment

dimaanj Aug 31, 2021

Choose a reason for hiding this comment

dimaanj commented Jul 22, 2021 •

edited

Loading