Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement SvgLayersGroup and SvgEdgesLayer in plexus #419

Merged

Conversation

tiffon
Copy link
Member

@tiffon tiffon commented Jul 19, 2019

Which problem is this PR solving?

Adding the components for rendering edges to the budding layer functionality in plexus.

Short description of the changes

Add components for rendering edges to plexus:

  • SvgDoc - An SVG document, this will be used by other components
  • SvgLayersGroup – A group of layers, right now it only supports the edges layer
  • SvgEdgesLayer – A layer of edges
  • SvgDefEntry – The definition of markers, for arrow heads on the end of an edge (or at the start)
  • SvgEdge – The edges, themselves

Also, add the util type TNonEmptyArray.

Examples from the demo

↓ Layer group version

Using a layer group to render visible edges and then a layer of edges above that layer with a wider but transparent stroke to catch mouse events over a larger UI element.

<LayeredDigraph
  zoom
  minimap
  className="DemoGraph--dag"
  layoutManager={new LayoutManager({ useDotEdges: true })}
  minimapClassName="Demo--miniMap"
  setOnGraph={layeredClassNameIsSmall}
  measurableNodesKey="nodes"
  layers={[
    {
      key: 'nodes-layers',
      layerType: 'html',
      layers: [
        {
          setOnNode,
          key: 'nodes',
          measurable: true,
          nodeRender: getLargeNodeLabel,
        },
      ],
    },
    {
      key: 'edges-layers',
      layerType: 'svg',
      defs: [{ localId: 'arrow-head' }],
      layers: [
        {
          key: 'edges',
          markerEndId: 'arrow-head',
          edges: true,
          setOnContainer: scaleProperty.strokeOpacity,
        },
        {
          key: 'edges-pointer-area',
          edges: true,
          setOnContainer: { style: { cursor: 'default', opacity: 0, strokeWidth: 4 } },
          setOnEdge: (...args) => ({
            onMouseOver: () => this.onEdgeEnter(args[0]),
            onMouseOut: () => this.onEdgeExit(args[0]),
          }),
        },
      ],
    },
  ]}
  {...largeDag}
/>

Screen Shot 2019-07-19 at 6 53 28 AM

Screen Shot 2019-07-19 at 6 53 45 AM


↓ Standalone layer version

Configuring the graph using an edge layer, directly, without a wrapping SvgLayersGroup.

Note: If there is more than one layer it's more efficient to use the group so the layers are transformed all at once instead of one at a time (for zoom).

Note: This version uses neato edges.

<LayeredDigraph
  zoom
  minimap
  className="DemoGraph--dag"
  layoutManager={new LayoutManager({ useDotEdges: false })}
  minimapClassName="Demo--miniMap"
  setOnGraph={layeredClassNameIsSmall}
  measurableNodesKey="nodes"
  layers={[
    {
      setOnNode,
      key: 'nodes',
      layerType: 'html',
      measurable: true,
      nodeRender: getLargeNodeLabel,
    },
    {
      key: 'edges-visible-path',
      defs: [{ localId: 'arrowHead' }],
      edges: true,
      layerType: 'svg',
      markerEndId: 'arrowHead',
      setOnContainer: [{ className: 'DdgGraph--edges' }, scaleProperty.strokeOpacity],
    },
  ]}
  {...largeDag}
/>

Screen Shot 2019-07-19 at 6 54 01 AM

Screen Shot 2019-07-19 at 6 54 17 AM

Signed-off-by: Joe Farro <joef@uber.com>
Signed-off-by: Joe Farro <joef@uber.com>
Signed-off-by: Joe Farro <joef@uber.com>
Signed-off-by: Joe Farro <joef@uber.com>
Signed-off-by: Joe Farro <joef@uber.com>
@tiffon tiffon added the plexus Specific to packages/plexus label Jul 19, 2019
@tiffon tiffon requested a review from everett980 July 19, 2019 11:00
@codecov
Copy link

codecov bot commented Jul 19, 2019

Codecov Report

Merging #419 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #419      +/-   ##
==========================================
- Coverage    91.6%   91.58%   -0.03%     
==========================================
  Files         176      176              
  Lines        3991     3991              
  Branches      920      920              
==========================================
- Hits         3656     3655       -1     
- Misses        297      298       +1     
  Partials       38       38
Impacted Files Coverage Δ
...s/jaeger-ui/src/components/SearchTracePage/url.tsx 80% <ø> (ø) ⬆️
...eViewer/TimelineHeaderRow/TimelineViewingLayer.tsx 88.13% <0%> (-1.7%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33994f0...2a2ba4e. Read the comment docs.

Copy link
Collaborator

@everett980 everett980 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly minor points, but one more significant question about how a pure component wrapper would affect setOnEdge in SvgEdgesLayer.tsx. That question is about a TODO so it doesn't affect this PR per se, but may impact the next PR.

packages/plexus/src/LayeredDigraph/types.tsx Show resolved Hide resolved
packages/plexus/src/LayeredDigraph/SvgDoc.tsx Show resolved Hide resolved
packages/plexus/demo/src/index.tsx Show resolved Hide resolved
setOnContainer: { style: { cursor: 'default', opacity: 0, strokeWidth: 4 } },
// eslint-disable-next-line no-console
setOnEdge: (...args) => ({
onMouseOver: () => this.onEdgeEnter(args[0]),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you only want one arg, why not remove the rest from above and the [0] from here?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or does TS complain if the length of the prop doesn't match?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. Will do in the next PR.

packages/plexus/demo/src/index.tsx Show resolved Hide resolved
@tiffon tiffon merged commit ed3cc72 into jaegertracing:master Jul 22, 2019
@tiffon tiffon deleted the layered-digraph-svggroup-edgeslayer branch July 22, 2019 18:43
@tiffon tiffon restored the layered-digraph-svggroup-edgeslayer branch July 22, 2019 18:43
vvvprabhakar pushed a commit to vvvprabhakar/jaeger-ui that referenced this pull request Jul 5, 2021
Implement SvgLayersGroup and SvgEdgesLayer in plexus
Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plexus Specific to packages/plexus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants