Create Workflow component with Lumino

[kinow]

These changes close #90, and close #323

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).
• Appropriate tests are included (unit and/or functional).
• Appropriate change log entry included.
• No documentation update required.

[kinow]

I was just going to get my toes wet learning Lumino, then when I realized it wasn't really hard decided to start digging in the TypeScript code (no docs) and examples. Tried to match as much as possible the design sketch.

There are some limitations to how much we can tweak due to the way it builds screens with multiple widgets, each having some logic to decide the height/width. For instance, I wanted to increase font size and give it more space (padding/margins) to the tabs, but this is quite complicated without docs/examples.

One major issue now is reactivity. I am attaching the Vue component, and destroying it (I think, hard to spot the exact object in the browser memory, but will confirm later). But the component is not being updated. i.e., here's what it looks like at the moment:

So each time I add a new Tree component, it picks up the current workflow status, then freezes. Not being updated even though Vuex data is still changing. So I've got to sort this out before this is ready for review.

Then need to check which commits need to be squashed, check if it's possible to add any tests (don't think so at this stage), add the changelog, and it should be ready for review 🎊

[kinow]

Also, almost forgot, not sure if we can use the Graph component. In order to add the Graph here, we must be able to create the Graph component programmatically.

If the Graph component (not the view) depends on VueRouter, extra objects, or if we are not able to create multiple instances of the Graph component, then we will need to refine the component code. But once that's done, adding a new Graph Widget (Lumino layout uses widgets, not components) shouldn't be too hard (<1 day work, couple hours perhaps or less).

[kinow]

Eureka moment going for a walk. Insteas of creatijg a new tree component myself, it should be possible to instead simply add some value to an array, then use a v-for to get vue to create the component for me.

May have to hide the component, or find a way to tell vue where it should go in the dom tho. But a possible fix for reactivity.

[kinow]

Came back from NIWA x-mas function and had to finish it otherwise I wouldn't be able to enjoy the weekend 😄

Next week will review the commits, add changelog, and then wait until @oliver-sanders or somebody else has time to review the current draft, before adding any more code. But so far, I'm really happy with Lumino 👍

[hjoliver]

That looks brilliant, well done 💐

[kinow]

Ready for review. Will try to add a few unit tests, but that shouldn't alter the current code, so happy to take any reviews.

The last commit alters the Graph component. I changed as little as I could to get it working as a Lumino widget, and as a standalone Vue component. Tested the existing Graph view, and it appears to be working fine, but better if reviewers can check it too.

Tested with latest Chrome/Ffox on Ubuntu LTS, and IE Edge on Win10Pro. Here's what it should look like:

A few known issues:

• Adding two graphs, only one is updated

• Adding graphs only works on Chrome and Ffox, IE almost works. In IE, I can see the graph in the mini-screen at the bottom right, but not in the main graph area

• There is an increase in memory usage. We will need to keep refining the code incrementally later.
• Occasionally, you may see an error in the console saying something like "Cannot access name of undefined", this is caused by families and tasks not matching in the returned GraphQL data. I will need to isolate the issue, reproduce, and create an issue either in Cylc UI or in Cylc UI Server to handle that.

The issues above are not blockers IMO, as they are not directly caused by the new Workflow component.

[kinow]

NB: No CI on this PR as #324 is still pending review 👍 but just ran lint/tests/build locally and it worked fine on my environment.

[kinow]

Unit tests added, change log too. Commits are as organized as I could, without re-writing code. They are more or less grouped, there some CSS/style commits that could have been grouped together but I worked on separate parts of the code while trying to integrate Lumino, sometimes on the JS part, sometimes on the CSS.

Tested again on Ffox/Chrome. Ran the CI commands locally and everything passed. The CI passed on my repository when I pushed it, so it's possible to confirm it should have passed here too: https://github.com/kinow/cylc-ui/actions (only not running because we need #324 in order to trigger the action on pull requests and on push).

Coverage is not being uploaded in the action. It will reduce, probably, as we have some code for a view added here that is not being tested, only the component is. Will fix the coverage in another PR.

[oliver-sanders]

Nice!

This is one of those really big things which brings the goal into closer view.

Few niggles:

• I've got a font issue on the close icons:
  
• The graph view currently isn't showing anything for me (no console traceback).
• Resizing doesn't seem to work.

[kinow]

I've got a font issue on the close icons:

Ah, good to know. I chose a special character for the X close button, but should have done the simple first. I'm pushing one commit to use a simple letter X now.

The graph view currently isn't showing anything for me (no console traceback).

Maybe if you try on a different browser. Though I am afraid the graph component needs a few more changes in order to be used as a widget with lumino (and we need to run a few more tests with cytoscape/etc, as IE Edge is not working, though I believe I've used Cytoscape apps with IE before)

Resizing doesn't seem to work.

This is a bit tricky with lumino. If you position two widgets side by side, then you should be able to resize their width, respecting a minimum-width that I set (otherwise when opening on mobile or smaller screens you wouldn't see the widget content.

But as for height, the hard limit is the screen size I think. Initially I don't set a height for lumino widgets. I set a min-height, and then it computes the widget height. I couldn't pinpoint exactly where they are doing that in their TypeScript code... but the best I could do was to change the height of the content div to occupy as much as possible (see screenshot below) and also define a min-height.

(content is the whole area, which includes the toolbar and the workflow area, as well as footer, that's why it has 100 height of viewport)

[oliver-sanders]

This is a bit tricky with lumino

Sorry, I meant resizing the "panes" within Lumino by hovering over the gap between panes until you get the arrow icon, then using click+drag.

[kinow]

This is a bit tricky with lumino

Sorry, I meant resizing the "panes" within Lumino by hovering over the gap between panes until you get the arrow icon, then using click+drag.

Ah, if that's working in JupyterLab maybe we can copy it from there. I used their examples to implement this PR. Here's how it works in one of their examples.

It should be working similarly in this PR.

[kinow]

Will try to fix the conflicts here today.

[kinow]

Will fix conflicts here after #199 and #280 👍

[kinow]

PR updated. Conflicts fixed. Had to update the GraphWrapper component. This component simply gets the required parameters to create a Graph component from the Add Tree dialogue, plus data from Vuex store, and creates a Graph to be added as a Lumino widget/Vue component.

Tested with the recently merged subscriptions/websockets, and two workflows. Almost everything seems to be working as expected.

There is one thing that I may fix after the Tree component. If you have 2 workflows, and you open one and add some views (e.g. one tree, and one graph).

Then, when you switch to another workflow, the two views are kept, and the tasks/jobs are replaced by the new component. Here's what I mean.

Between bug and feature, I think this probably falls in the category of bug/unexpected behaviour.

[kinow]

Argh, one test failing 😠

[kinow]

GitHub action happy. This is officially ready for review! Will keep an eye for comments/reviews, but focusing now on the treeview as holiday homework (read: nerdy fun).

[oliver-sanders]

Resizing Lumino tabs is now working brilliantly!

I think there is an issue with padding, the whitespace under the toolbar appears to be associated with the <main> element. The whole page is vertically scrolling by a similar amount so that might be related.

This could be booted to a follow-on PR.

[kinow]

Squashed commits 👍 but still pending one item from Oliver's review.

[kinow]

Fixed #323 while testing this PR with the "Add View" link.

[kinow]

Finished moving the subscriptions to the view. The Workflow component reacts to events, and adds new widgets. When a widget is added - alongside a Vue wrapper - we also create a subscription.

The widget has the same ID as the subscription. Later, when the user closes the widget, we capture the Lumino event, and emit a new Vue event. That event then triggers the deletion of the Vue components (wrapper et al) and the removal of the subscription.

This a compromise for the discussion around where to keep subscriptions.

The components are free of GraphQL details or queries. The subscription code (gquery in special) is not touched, and works as per design. Screenshot showing one way to confirm what subscriptions we have:

Final notes: this PR has a commit that I picked from the Infinite Tree PR to fix the offline mode. Whichever goes first, the other PR will need amending. It is not really important, as they will cause conflicts anyway in the tree component. The conflicts will take a couple of days for fixing and testing, as I haven't experimented the infinite tree with lumino 👍

[oliver-sanders]

Okay, so you've copied the queries from the tree and graph view into the workflow component?

If so should the query in src/views/Graph.vue still be there?

[oliver-sanders]

Okay the "compromise" makes sense, it restores the "only subscribe to what you need" model, further refinement should be attempted separately.

[kinow]

Okay, so you've copied the queries from the tree and graph view into the workflow component?

If so should the query in src/views/Graph.vue still be there?

Replied on Riot, copying here for reference

Yes, I think @hjoliver mentioned he'd like to keep the old table at /workflows working. That one where users can see a listing of workflows, and under Actions I think there are links for the Tree view and for the Graph view.

[oliver-sanders]

Woohoo, looks good, works, all happy.

[oliver-sanders]

I think this is perfectly usable and further enhancements / tweaks can be punted as this is already a sizeable change - hence #358 which we can live with until 0.3.

[hjoliver]

💯