[Graph] Reactify visualization#46799
Conversation
flash1293
added
Feature:Graph
|
Pinging @elastic/kibana-app |
💚 Build Succeeded |
|
@elasticmachine merge upstream |
💚 Build Succeeded |
kertal
left a comment
kertal
left a comment
There was a problem hiding this comment.
Code LGTM. Testing it with chrome I noticed warnings in console:
Good point, this was caused by the ace editor in the inspect panel (this will go away shortly and be replaced by the standard inspect panel). I fixed it temporarily by doing what the warning says :) |
💚 Build Succeeded |
kertal
left a comment
kertal
left a comment
There was a problem hiding this comment.
Code LGTM 🖼 , no more warnings
Just a notice while I tested und unrelated to this PR, you could change CSS to show a pointer while hovering a node
| .gphNode__text { | ||
| fill: $euiColorDarkestShade; | ||
|
|
||
| &--lowOpacity { |
There was a problem hiding this comment.
Remove this class completely. The icons should be fully opaque when on top of the node colors.
There was a problem hiding this comment.
Good point. This class was still used in other places because the styling of icons in menus was entangled with the node styles. I separated them and removed some unused classes.
| } | ||
|
|
||
| .gphNode__text { | ||
| fill: $euiColorDarkestShade; |
There was a problem hiding this comment.
These fills need to be always black or always white, not switchable via theme. So that when going from
to dark mode, you get
Instead of
| fill: $euiColorDarkestShade; | |
| fill: $euiColorInk; |
There was a problem hiding this comment.
Good point, fixed. As the visualization now supports dark colors, I removed the light color filter from the default palette.
| } | ||
|
|
||
| &--inverse { | ||
| fill: $euiColorLightestShade; |
There was a problem hiding this comment.
| fill: $euiColorLightestShade; | |
| fill: $euiColorGhost; |
💚 Build Succeeded |
cchaos
left a comment
cchaos
left a comment
There was a problem hiding this comment.
A couple more suggestions but then it LGTM
| opacity: .7; | ||
| } | ||
|
|
||
| &--pickable:hover, &--pickable:focus, &--selected { |
There was a problem hiding this comment.
Separate selectors to their own lines.
| } | ||
|
|
||
| .gphNode__text { | ||
| fill: $euiColorInk; |
💚 Build Succeeded |
5 participants
This PR migrates the visualization itself to React and adds some tests. This is an interim solution, it will eventually be implemented with cytoscape.js: #46784
The resulting DOM should stay exactly the same, with a few renamed classes to match BEM and light icon colors for dark node colors.
@miukimiu @cchaos If you have some quick win ideas to improve the appearance they would be appreciated - it's not required for this PR though.