Basic layout process nodes and edges

[peluja1012]

Summary

Renders basic layout of nodes and edges. See https://github.com/elastic/endpoint-app-team/issues/78

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

[bkimmel]

❓ I think <line x1="${startPosition[0]}" x2="${${startPosition[0]}" y1="${endPosition[0]}" y2="${endPosition[0]}"> would be easier to read and deal with. Forcing these things into HTML/CSS space seems like it makes them a little resistant to some of the things we could do potentially do later to differentiate and polish this product as it matures (e.g. using paint servers for the stroke of the line, filters, masks, maybe animating parts of the transform). @oatkiller I know you said you wanted to defer the conversation until we started "rendering" things - and it looks like that's what we're starting to do here, right? I'm OK if we decide as a team to just use <div>s and <span>s with CSS applied, but I still feel like a system that is specifically built in the Web to handle positioning and rendering things in a Cartesian coordinate system (like we're trying to do) is a better fit here. As fun as it is to see -Math.atan2 in code we use, I think a casual viewer of the code seeing "x1, x2, y1, y2..." could be a little more confident about how it's working and what they change. I acknowledge the downsides are 1) people aren't as familiar with SVG, so maybe that would defeat the whole purpose? 2) IE11 doesn't do CSS transforms in SVG space, so we'd be stuck with it. 3) SVG has more cross-browser issues. But I think the advantage of SVG being vastly more powerful visually and it being easier to "express" the kinds of things we're trying to do in coordinates makes it a better fit for the long game of the Resolver.

[oatkiller]

seems worth investigating but probably out of the scope of this PR (unless that decision would change the graph layout logic.)

If you're interested in using SVGs here, you could make some progress by proving that we can nest any embeddable we want inside of svg?

[oatkiller]

in any case, we'd still have atan and what not. changing a div to a line isn't going to change the math of laying out the graph right?

[bkimmel]

Using the line obviates the need for aTan and other math: You give it coordinates of where it should start and end and it lays itself out.

[bkimmel]

Which is a case in the overall point I'm making: It's 1 small thing here, but these complexities seem likely to "pile up" as we do more and more along the path of trying to torture CSS flow-based layouts to meet the coordinate-based needs of Resolver.

[bkimmel]

@oatkiller w.r.t. the question of embedding HTML-based DOM elements in SVG, ForeignObject ( demonstrated here: https://bl.ocks.org/Jverma/2385cb7794d18c51e3ab ) is reported with support across most UAs (with the notable exception of Android for some reason: https://caniuse.com/#feat=mdn-api_svgforeignobjectelement )

[oatkiller]

now that we have typescript, I stopped putting my arguments in objects like that. typescript can give you hints as to what each is, along w/ inline documentation. also it'll catch you if you're wrong in most situations. plus you can automatically refactor it across the whole project.

[oatkiller]

as far as types go, something like this should work. I think the iterator type should be implied.

export function* depthFirstPreorder<T>(root: T, children: (parent: T) => T[]) {

[oatkiller]

Here's a version that does away with the object wrappers in the arguments and has types:

export function* depthFirstPreorder<T>(root: T, children: (parent: T) => T[]): Iterable<T> {
  const nodesToVisit = [root];
  let currentNode
  while (currentNode = nodesToVisit.shift()) {
    nodesToVisit.unshift(...(children(currentNode) || []));
    yield currentNode;
  }
}

export function* levelOrder<T>(root: T, children: (parent: T) => T[]): Iterable<T> {
  let level = [root];
  while (level.length !== 0) {
    let nextLevel = [];
    for (const node of level) {
      yield node;
      nextLevel.push(...(children(node) || []));
    }
    level = nextLevel;
    nextLevel = [];
  }
}

[peluja1012]

Thanks. Done.

[oatkiller]

I think we could make this logic easier to read/understand once we've added some tests. Maybe we do it like:

1. port it over (you did this)
2. test it
3. refactor it until its clean and nice while keeping tests passing

[oatkiller]

Check out this comment I made on Paul's PR: https://github.com/elastic/kibana/pull/53906/files#r363367614
I explain my ideas around organizing selectors. Can we do that here? Basic plan:

1. Use a reducer just for this concern, let's call it dataReducer for this example. It takes DataState and ResolverAction and returns a DataState. This reducer doesn't know anything about global state.
2. Have 'data' selectors that take DataState. They don't know anything about global state. These can be called by the dataReducer since the data reducer takes DataState and these selectors take DataState. These are sorta like model methods for dataState
3. Import and re-expose some data selectors from store/selectors. The selectors in store/selectors take Global state and are the only selectors used by react components. This is because react components receive global state from useSelector, and they should have no knowledge of the internals of the global state. Since the 'data' selectors take 'DataState' and not global state, you will need to create a version of your selectors that takes GlobalState and then calls the underlying data selectors with the right piece of it. I'm using a helper i made called composeSelectors. Here it is in my PR: https://github.com/elastic/kibana/pull/53619/files#diff-7dd777f1d03cd310741b16bdfb94291fR28
4. Use only the selectors in store/selectors from react components. These selectors can be thought of as 'public' or you could think of them as model methods for GlobalState.
5. If some business logic isn't really specific to any particular state, but instead operates on a well defined business construct (e.g. Event, ProcessEvent, User) then place that logic in models/whatever. These types of functions can be pure (they don't rely on state per say, just an entity that could be found anywhere.) They should be easy to test as well, without state there is no need for redux/actions/etc.

[peluja1012]

Done

[oatkiller]

the readability of this stanza might be improved if Vector2 had a distance and an angle method.

import { angle, distance } from '../lib/vector2'
// ...
const length = distance(startPosition, endPosition)
const angle = angle(startPosition, endPosition)
// ...

[oatkiller]

can you change this to unknown or some more specific type? With the introduction of unknown and never, we should be able to avoid any (in 99.999% of cases. its an open debate in typescript if a third generic type is needed haha)

[peluja1012]

Changed it to ProcessEvent type.

[bkimmel]

These look good, but I think having "nodes" in the name of the function would make it easier to understand (like nodesByLevelOrder or levelOrderNodes)

[bkimmel]

❓ This is just a placeholder while it gets developed, right?

[bkimmel]

❓ If we ever get to the point where we want to graph things other than processes, we could change the name of this.

[bkimmel]

❓ maybe call them eventType and eventSubtype to keep things tidy

[bkimmel]

❓ For readability, explaining some of these terms like "source" and "target" would make understanding this for a new reader easier

[bkimmel]

❓ In comments, some explanation of why you are doing this would be helpful to the reader.

[bkimmel]

❓ Have you tested making this more than 1? Some ideas Lindsay and I are working on use space from a node's "descending" edge-line to lay out information. Would be super cool if this works (I assume it does).

[bkimmel]

Nice. Other than the "big question" of how we want to render these things (in HTML or SVG space) and being more descriptive with better names and comments in a coupe places, this looks to be in order.

[oatkiller]

Would you mind putting comments on all exported stuff? For example you could write:

/**
 * Sequences a tree, yielding children returned by the `children` function. Sequencing is done in 'depth first preorder' fashion. See https://en.wikipedia.org/wiki/Tree_traversal#Pre-order_(NLR)
 */
export function* depthFirstPreorder<T>(root: T, children: (parent: T) => T[]): Iterable<T> {

[peluja1012]

done

[oatkiller]

want to pair on this?

[oatkiller]

why not ResolverAction here?

[peluja1012]

Changed it to DataAction. I forgot to change it after creating the DataAction type.

[oatkiller]

These values won't be redefined before each test. Maybe have let processA, processB, etc in the describe function body and then processA = {...}; processB = { ...} in a beforeEach so they are redefined?

[peluja1012]

Good call. Done.

[peluja1012]

Closing this PR. This code is not included in the following PR: elastic#53619.