graph with live data and node info

[MartinRyan]

graph with node info
supersedes #241

PULL REQUEST
Requirements check-list

• I have read CONTRIBUTING.md and added my name as a Code Contributor.
• Contains logically grouped changes (else tidy your branch by rebase).
• Does not contain off-topic changes (use other PRs for other changes).
• Already covered by existing tests.
• No change log entry required (DRAFT).
• No documentation update required.

[hjoliver]

@MartinRyan - trying this out, I see the labels and job icons at all zoom levels, but I only see the node icons when zoomed in very close. Do you see that?

[MartinRyan]

@MartinRyan - trying this out, I see the labels and job icons at all zoom levels, but I only see the node icons when zoomed in very close. Do you see that?

I see them at all zoom levels

[hjoliver]

I wonder if it is some caching issue. I see the node icons after a reload, but a bit of zooming causes them to disappear again.

[MartinRyan]

I wonder if it is some caching issue. I see the node icons after a reload, but a bit of zooming causes them to disappear again.

I've tested in chromium, firefox and chrome on linux

[hjoliver]

Reinstalling and rebuilding the UI from scratch, just in case.

[kinow]

Just tried on Firefox LTS, Ubuntu LTS, working for me 👍

(I moved some nodes, then the browser refreshed it and they were re-organized, then the GIF ends 1 second after that... but it's being updated alright)

[hjoliver]

@kinow - did you do a bunch of zooming, with the +/- controls?

[kinow]

@kinow - did you do a bunch of zooming, with the +/- controls?

Randomly, but just tried again focusing on zooming while it was updated, with no issues.

[hjoliver]

Looks good - I want that! (Will try what you suggested on Riot...)

[hjoliver]

I had a broken Google Chrome - downloaded the latest and its working properly now 👍 phew.

[hjoliver]

All good now, even with quite large suites 🎉

Treeview styling is affected on this branch:

(coloured backgrounds around all the task and job icons?)

On master:

[hjoliver]

As discussed on Riot, I vote to merge this soon. Minimal changes needed are:

• fix whatever is breaking the tree-view styling (screenshot above)

• remove the Cola layout, at least for now

  • at first click it seems to adjust the existing layout, with a settling down animation; then from second update onward it is a total mess (not sure what's going on there?)

• I'm not sure about Cose-Bilkent; it might be nice for some things - it tends to group cycle points (at least for my current test suite). Does it have the settling down animation? Something weird seems to happen at each update, in terms of transitioning to the final state. Ah - it breifly reduces to a single node during each update - that would need to be fixed.

[hjoliver]

The freeze button is really good, and users probably will need it when inspecting the graph closely (as opposed to watching its evolution) but it forces the dagre layout.

[hjoliver]

Gif showing cose-bilkent single-node update jump, and freeze -> dagre behaviour:

[MartinRyan]

fixed the cross styling and freeze button issues

[MartinRyan]

Do we need to support changing graph layouts now? If not, could we copy this branch somewhere else for reference later, and use a single layout? This would reduce the code of the component by a good amount (helpful in case we need to change it before adding more layouts, or use/test other graph libs)

I think they are useful, but perhaps others can decide that

[MartinRyan]

Some JS code is not used, sometimes because props are always set to false, not ever changing. Could we remove this code? It appears to be left-overs from previous code/tests, or other work-in-progress that can probably be done later.

yes, hopefully I've removed that now

[MartinRyan]

Two files with conflicts @MartinRyan. Can we also update the description of the issue at the top ☝️ to remove the wip related text?

done

[MartinRyan]

I think we don't need another service to fetch the data. The design proposed by Oliver for the workflow service should be able to merge queries, and serve data (via polling for now) for all views & components. This would make it easier to switch to the subscription service later on, plus, it works with offline/mocked data - there's a global alias already defined in vue.config.js.

I'm now using the workflowService, though the 5 second update is a little short for the graph. I was using a 10 second interval on the cytoscapeService, perhaps a 10 second interval would be better for the workflow service?

[oliver-sanders]

Hi, I think there is something wrong with the package-lock.json (causing the build failure).

I removed and rebuilt the lock but an getting this error in the console:

updateGraph error:  TypeError: "cy is undefined"
    _callee12$ Graph.vue:1139
    Babel 9
    Lodash 3

Perhaps the same issue.

[MartinRyan]

Hi, I think there is something wrong with the package-lock.json (causing the build failure).

I removed and rebuilt the lock but an getting this error in the console:

updateGraph error:  TypeError: "cy is undefined"
    _callee12$ Graph.vue:1139
    Babel 9
    Lodash 3

Perhaps the same issue.
I'll take a look

[oliver-sanders]

Hi @MartinRyan can you explain the need for all of these has(edge, <property>) statements?

This code seems to work fine for me:

        each(edges, (edge, key) => {
          edgeObj = {
            data: {
              id: edge.id,
              source: edge.source,
              target: edge.target,
              label: edge.label,

[MartinRyan]

The functions workflowUpdated() getEdges() and getNodes() construct the required cytoscape data format and are a workaround until the graph is supplied with the correct flat data, then they can be removed and the data supplied to the updateGraph() function. This will really speed performance back up again.
These sanity checks prevent the graph breaking if bad data is supplied, obviously if this is never the case then they aren't needed.

[codecov-io]

Codecov Report

Merging #289 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #289   +/-   ##
=======================================
  Coverage   62.85%   62.85%           
=======================================
  Files          20       20           
  Lines         315      315           
  Branches       17       17           
=======================================
  Hits          198      198           
  Misses        113      113           
  Partials        4        4

Impacted Files	Coverage Δ
src/services/gquery.js	53.62% <0%> (ø)	⬆️
src/components/cylc/Task.vue	100% <0%> (ø)	⬆️

---

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 f6a4921...d3efd1a. Read the comment docs.

[kinow]

For posterity, this PR has a change in package.json, adding svgo 1.3.2 as devDependency due to svg/svgo#1180

That dependency can be later removed, if vue-cli is updated and includes this dependency or fixes the issue with npm run build in some other way.

[MartinRyan]

Some CSS classes are being defined as top level elements, and may conflict with other elements in the UI (e.g. #holder div has several buttons and other HTML elements as children, that have their own classes like .frozen-button... I think it would be safer to move these styles under #holder if possible, or some other .cy-graph-something-etc parent element)

css made more specific

[kinow]

Errors in CSS/JS were fixed, Travis is happy, LGTM - and move other issues to follow-up PR's 👍

[MartinRyan]

corrected css class name

[oliver-sanders]

OK, let's get this in and refine later. Thanks @MartinRyan.