Closed oliver-sanders closed 2 years ago
Taking a quick look around I think we are unlikely to find a tree which will marry into our data structure as our data is kinda unstructured in a way, its about how you combine families, cycle points, etc which forms the tree in Cylc.
Unless anything crops up which ticks all the boxes I'd be inclined to nick the Vuetify CSS and apply it to a <ul />
tree to give the composer complete control over the tree structure:
<ul v-for="x in y">
<li>
<ul v-for="j in j">
<li>
....
Which is the approach used in https://github.com/cylc/cylc-ui/blob/master/src/views/Tree.vue
The Vuetify tree is about 400 lines of typescript so really not that large a development effort https://github.com/vuetifyjs/vuetify/blob/3a9fdc292af97c98be91c014bde1fac40a96e865/packages/vuetify/src/components/VTreeview/VTreeview.ts
With Vuetifyjs 2.0, the TreeView appears to have gained a few more options. The component has a demo playground where we can change the settings and see what it would look like on the fly.
Component | 1 | 2 | 3.1 | 3.2 | 3.3 | 4 | 5.1 | 5.2 | 5.3 | 6 | 7 |
---|---|---|---|---|---|---|---|---|---|---|---|
Vuetify Treeview | :ok: | :ok: | :ok: | :no_entry: | :no_entry: | :no_entry: | :no_entry: | :no_entry: | :no_entry: | :no_entry: | :no_entry: |
selectable
option fits here? It displays a checkbox to select any element in the tree. The last example in the new component (breweries) allows you to customize the selectable icon, and you can choose multiple entries.But looking at the other items, it's still the same as before. I don't think we will find one that checks all the boxes here.
I thought we would have to write a component from scratch (especially because of the responsive part), but if we can get some parts of TreeView that'd be great. I started a small experiment to use the List
+ Breadcrumb
for the mobile responsive part, and TreeView
for the normal tree, but unfortunately my mind is not quite sharp, and I got stuck with the Codepen small panels, trying to display the breadcrumb :)
I'm using version 2.0.2 of Vuetify, and their TreeView is a bit limited. No easy way to disable/gray out items (as in tasks already done as per sketch). We will need to customize it quite a bit I think. But we may be able to just create a new component based on this one perhaps... we don't need to use TypeScript... we can eliminate the features that we don't need, and just copy whatever we need.
@oliver-sanders are you planning to work on this? Otherwise I can have the first try at creating a simple tree component. The part of tabular data is not so clear for me, but the rest I think I understand how/why we need.
The part of tabular data is not so clear for me,
@kinow - do you mean how to do the tabular data (in a tree view) is not clear, or why we need it?
On why, I can see @oliver-sanders 's point that the tabular form is useful if you want to sort a whole bunch of jobs by submit-time (e.g.). But it is unfortunate that we need it from a clean-layout point of view.
I suppose the question is then, can we get (or make) a single treeview component that can do it all - can it switch between tabular and tree-expand at the lowest (individual task) level - or do we need two views? I think @oliver-sanders thinks we can do it with one, but perhaps that remains to be seen.
If it does prove difficult to do it all with a single tree-view, I think another option is:
As an example of 2. see the cylc-7 GUI tree view with family grouping turned off. Such a view could still be family-friendly by allowing filtering (inclusive and exclusive) based on family names.
I'm expecting this idea not to be very popular with the UK team (ping @oliver-sanders?) but I think we should consider it. Advantages:
The only(?) disadvantages are:
@cylc/core - any opinions on this?
@kinow - do you mean how to do the tabular data (in a tree view) is not clear, or why we need it?
I am more confused with the how. I thought we would be able to display extra information in a collapsible panel, which appear in the sketch. Where the user would click a task/family/or workflow to see more information e.g.
I think the information in this panel is the same we have in the current table?
I got the impression after the last meeting that some users preferred to have the table. What I had in mind for that, then, was to use two components allowing the user to switch back and forth between then.
That's how NIWAData does when it displays a graph with information for the users, but there is a button at the top that when the user clicks, it turns around and has some other information in the back, https://data.niwa.co.nz/#/what_you_get
But I like your suggestion of simply having two components, and let the user choose. +1 especially for "less complexity in our own code base".
The only(?) disadvantages are:
- ...
- no family collapse/expand in the tabular form
Why can't we have a table with expand/collapse option for families? I think the vue-ads-table-tree component was able to show tabular data with hierarchy, allowing to extend/collapse table rows.
We can always split it into two components, that's not a huge problem.
I don't see it as being a big challenge though, getting a table layout in CSS is trivial these days.
<!-- psuedo-vue -->
<ul>
<li>
<task status="task.status">
{{ task.name }}
<span if="expanded">
<span for="column in table">
{{ column}}
</span>
</span>
</li>
</ul>
@oliver-sanders are you planning to work on this? Otherwise I can have the first try at creating a simple tree component.
Feel free to take a crack at it, I think you have a much better understanding of Vue than me!
Feel free to take a crack at it, I think you have a much better understanding of Vue than me!
Not sure about that, but I intend to check with you as soon as possible once I have anything (especially CSS; the Task & Job components got well organized scss structure).
On the multi-selection, just to confirm: as a user, can I select multiple tasks, from multiple workflows? Or is there any requirement to allow multiple selections only within the same workflow?
On the multi-selection, just to confirm: as a user, can I select multiple tasks, from multiple workflows? Or is there any requirement to allow multiple selections only within the same workflow?
There is actually a requirement for users to be able to select multiple tasks across suites (workflows). E.g. To hold all tasks that run on a certain job platform and the job platform is about to go down for maintenance; or to re-run all failed archiving tasks (when the archiving system was down and is now back up again).
On the multi-selection
I think we at least need to implement the same multi-selection functionality we have in the Cylc7 GUI:
Multiple workflows
Why not!
I don't see any reason to restrict the number of suites displayed by a view to one, even if we don't make use of this functionality in early Cylc8 releases.
Good points, noted! Multi selection across workflows it will be then. Thanks!
Started with the approach from an existing component, where we re-use the same component recursively, i.e.
# file: TreeItem.vue
<template>
<div>
<div
class="node"
:style="{'margin-left': `${depth *30}px`}"
>
<span
v-if="hasChildren"
class="type"
@click="nodeClicked"
:style="getTypeStyle()"
>{{ expanded ? '▽' : '▷' }}</span>
<span class="type" v-else>◇</span>
<span>{{ getNodeName() }}</span>
</div>
<span v-if="expanded">
<TreeItem <--------------- Here's the recursion
v-for="child in node.taskProxies"
:key="child.id"
:node="child"
:depth="depth + 1"
:cycles="cycles"
></TreeItem>
</span>
</div>
</template>
The issue is that the structure of the workflows has taskProxies
the have all the task proxies, from multiple cycle points.
The current Tree.vue
that we have is fetching the workflows, then gathering the cyclepoints, and using that when iterating the workflows, in order to create the structure as in the design sketches.
@oliver-sanders just wondering if the data structure is going to change, or if the component will have to handle the cycle points? (I'm already working on it, but just in case there are any plans to organise the tasks by cycle points... as that would be way easier :) )
but just in case there are any plans to organise the tasks [in the data structure] by cycle points...
Just discussed on Riot chat - no plans to do that.
Still a bit clumsy, as selection values are not persisted (probably due the use of v-if
instead of v-show
[the latter uses display: none
, the former I believe removes from virtual-dom/DOM]). But here's what it is looking like:
Branch with the current code: https://github.com/kinow/cylc-ui/tree/tree-component-1
There are only three files that matter being touched.
views/Tree2.vue
- will be deleted, just a test view. It is very similar to the existing Tree.vue
, with the following differences:
Set
and iterating over for each workflow, it is using a Map
workflow
plus the computed cycles
Map
, it creates another computed structure workflowTree
components/Tree
: just iterates over each workflow in the workflow tree, and calls the TreeItem
components/TreeItem
: the component that actually renders the tree, and calls itself recursivelySo every entry in the Tree
is a TreeItem
component instance, with depth
, expanded
flag, and the node
.
With this approach, we should be able to control whether a node is expanded or not programmatically. We can have functions to collapse all tasks, or expand all cycle points, etc.
I will be working to add slots now. With slots, I hope to be able to allow to customize what the component does when we have a Task, a Cycle point, a Job, etc. Not sure if it will work OK due the recursion... but there's only way way to know it :-)
The ugly in this approach is the computed data structure, which is about the same size as the workflow. Vuejs should take care of updating it, but the approach in views/Tree.vue
doesn't require touching workflows
, only the extra cycles
Set
... but I couldn't think of a way of using that to build a component :disappointed:
Oh, and the reason for adding slots, is so that this component can be used in both Tree.vue
and GScan.vue
.
Still a bit clumsy, as selection values are not persisted (probably due the use of v-if instead of v-show [the latter uses display: none, the former I believe removes from virtual-dom/DOM]). But here's what it is looking like:
Fixed after using v-show
instead :+1:
Looking good!
Wow @kinow that was quick. Your approach looks sensible to me, nice and flexible. :clap:
@kinow Just took a look at your branch. Big result with so little logic!
Thanks guys! I can't take much credit. Parts of the logic are from an existing component, and having the existing Tree.vue
with a working version for reference helped a lot too. But really happy it's going in the right direction :+1:
I can't take much credit.
Please take some much-deserved credit Bruno! This looks great :100:
Added features to
The current implementation is based on the Vuetify TreeView component. Gif showing these features:
Some questions after looking at the current result and the sketch:
1.1 In the image below, next to the entries of a Workflow and of a Cyclepoint (highlighted) there are two icons. The Workflow icon appears to indicate that the suite is running. Is that correct? I reckon we don't have a component to use here, so will just copy the existing design from the Task
component but with a play button (a triangle + 90 degree transform? maybe some border?).
1.2. The Cyclepoint appears to use the Task
component, but with a summary for the cyclepoint? If so, what is the formula for setting the Task
state in the cyclepoint? Failure if any task failed? Success only if all the tasks succeeded? etc
I think in the sketch the jobs with highest submission number appear at the top. But I am displaying the data in the same order returned from GraphQL. Changing it is quite easy, but just wanted to confirm that we must display from the greater to the smallest?
I think that's the Job host?
If so, the task bar
, in the family BAR
, is collapsed. So we have the summary of the jobs (1, succeeded I think), and the text deepthought next to it. I am assuming this means this task had 1 job, that succeeded, on the host deepthought.
But what is the rule for the task baz
of the family BAR
when collapsed? It will have 2 icons, representing the two jobs, with their respective color/status. But one has the text anotherhost, and the other has asdf. Which value should be displayed?
The component now has the options to hover items, and to mark items as active. Probably we will have only one item active.
The Vuetify TreeView has the same features, plus the "selectable", which when enabled displays a checkbox next to each element, allowing the user to select one or multiple nodes.
I think we can implement the examples in this issue description, allowing the user to use SHIFT and CTRL to select multiple contiguous and non contiguous items... though implementing will take a bit longer (and generate a few more questions :)
But would it be alright to have this feature only in desktop mode? Or is there an alternative for mobile to allow multiple selection? I know some mobile apps allow you to Hold your finger on one item, and that activates multiple item selection... but I think that would be easier to be implemented later...
The Tree component appears to align job icons to the left when summarizing a Task:
While the GScan component seems to align job icons to the right when summarizing a Workflow:
Should we align both the same way, or keep left/right as they are?
Also, we will have to wrap when there are too many jobs... e.g. if a workflow has over 100 jobs, it won't be able to be displayed in GScan or Tree... we will have to display as many as possible - given any other information displayed like the task or workflow names - and wrap the remaining items. Is that OK?
ps: tried to use slots, which was supposed to be simple, but turns out it's not that simple when you have recursion in your component... interesting, but so far it's Vuejs 1 x 0 Bruno
1.1 In the image below, next to the entries of a Workflow and of a Cyclepoint (highlighted) there are two icons. The Workflow icon appears to indicate that the suite is running. Is that correct?
Yes, "play" (triangle) = running workflow.
Possibly we should have a whole-workflow state summary icon though (or as well) - what do you think @oliver-sanders ?
1.2. The Cyclepoint appears to use the Task component, but with a summary for the cyclepoint? If so, what is the formula for setting the Task state in the cyclepoint? Failure if any task failed? Success only if all the tasks succeeded? etc
We go through a list of statuses in order of importance/criticality, and stop at the first that is present in the suite. In 7.8.x see lib/cylc/task_state_prop.py:extract_group_state()
2 (order of jobs)
Most recent job at the top
- (host text)
Good question, let's see what @oliver-sanders was thinking there...
4 (multi-select)
IMO we can just do it for desktop mode, at least to start with
5 (job icon alignment)
I think the scan component is different in that it appears in a narrow-width sidebar, and the job icons are the right-most content in it. So it may make sense to right-align that, but not the tree component (esp. if there is other stuff to the right in the tree view, e.g. job host). (Or do you just mean right-align within a column?)
Also, we will have to wrap when there are too many jobs... e.g. if a workflow has over 100 jobs, it won't be able to be displayed in GScan or Tree... we will have to display as many as possible - given any other information displayed like the task or workflow names - and wrap the remaining items. Is that OK?
I think we discussed truncating the list of job icons - e.g. show just the five most recent jobs, then (and 95 more...)
or something like that.
To further elaborate on Hillary's comments.
- What icon should we have for Workflow and Cyclepoint?
Workflow:
Cyclepoint:
what is the formula for setting the Task state in the cyclepoint
The Workflow icon appears to indicate that the suite is running. Is that correct?
Yes
we don't have a component to use here
We can put one up, should be really quick as the standard icon set may suffice.
- What should be the order of jobs?
Most recent job at the top
- What is the text next to the Task summary and how to select its value?
Job host.
So this is the first column of the data table, perhaps it can be hidden by default.
image
I am assuming this means this task had 1 job, that succeeded, on the host deepthought.
Yes.
This comes from Dave who felt that we needed a way of collapsing jobs so that the tree view didn't become too unwieldy, which is a fair point. Perhaps we could collapse the job if only one has been submitted.
What is the rule for the task baz of the family BAR when collpased?
TBC!
Options:
- How should we handle multi-select?
Probably we will have only one item active.
Sounds sensible.
Or is there an alternative for mobile to allow multiple selection?
Should be! Good quality mobile support can follow later.
The alignment of jobs in the tree view (Cylc8) kinda bugs me. For a collapsed job we could potentially put the job icon next to the task icon? The potential for many-many relationship between tasks-jobs makes this messy.
If we have room to display a message for a job, I'll vote for the job host/platform + the job ID.
If we have room to display a message for a job, I'll vote for the job host/platform + the job ID.
I agree, that's definitely the most critical job info - where is the job, and what's its ID?
If we have room to display a message for a job, I'll vote for the job host/platform + the job ID.
In the sketches I left it as just the host/platform as the JobID is something you would only want to know if something has gone wrong (i.e. information you would be happy to request on a case by case basis rather than information you would need in tabulated form).
Progress of today:
Thanks for this tip @hjoliver
We go through a list of statuses in order of importance/criticality, and stop at the first that is present in the suite. In 7.8.x see lib/cylc/task_state_prop.py:extract_group_state()
It was exactly what I needed to get this done. But I left the bonus points mentioned by @oliver-sanders in a previous comment for later :grimacing:
Thanks @oliver-sanders
What should be the order of jobs?
Most recent job at the top
Done with a quick .reverse()
in the JS array for jobs.
@hjoliver you mentioned it on Friday while testing the branch. Took me a while to realize, but there was an error on my page, in the created()
method. I was using the old code from views/Tree.vue
, that gets the route name
param to subscribe the view... but for /#/tree2
there is no name in the route. Fixed by using a fixed value for the view ID, and tested it running a suite for a few hours.
I noticed that sometimes the tree completely disappears, but I haven't had time to investigate and see whether it's a bug in the component, or if the GraphQL API is returning an empty data structure.
Still a lot to do, but at least it's not stalled, still progressing well :+1:
Logged one issue for the log output, which is getting in the way while running the UI + backend https://github.com/cylc/cylc-flow/issues/3300
And another issue for exceptions in Cylc UI Server https://github.com/cylc/cylc-uiserver/issues/65
A nice and useful feature by Vue.js, is that as we are using v-for
with :key
, and the :key
is equal to each node's ID (cyclepoint, workflow id, task id, job id, etc), Vue.js caches the components based on the key. So if we collapse or expand a node in the tree, it will be kept the same way even after the data has been refreshed.
So if you have a cycle point that you don't care, you can collapse it and refreshes won't expand it again :tada: quite sure there was a bug about nodes being expanded again in the old GUI, so that should be fixed automatically thanks to Vue.js' caching
Tried the latest version, works well, looks good :+1:
Thanks @hjoliver !
Today I am updating the query to include the host
information, so that I can experiment displaying the host name near the job ID.
At the same time, thought about adding the code to display the task progress (may go in a separate and early pull request). @hjoliver pointed the code used in Cylc 7 to track the task progress in Riot (thanks again!):
The code resides in lib/cylc/gui/updated_tree.py:
meant = summary[id_].get('mean_elapsed_time')
tstart = summary[id_].get('started_time')
tetc_string = None
for dt in tkeys:
t_info[dt] = summary[id_][dt]
# Compute percent progress.
t_info['progress'] = 0
if (isinstance(tstart, float) and (
isinstance(meant, float) or
isinstance(meant, int))):
tetc_unix = tstart + meant
tnow = time()
if tstart > tnow:
# Reportably possible via interaction with
# cylc reset.
t_info['progress'] = 0
elif tnow > tetc_unix:
t_info['progress'] = 100
elif meant != 0:
t_info['progress'] = int(
100 * (tnow - tstart) / (meant))
Will JS-ify the code and see if that works, plus update the component GraphQL query to include the fields workflows / taskProxies / task / meanElapsedTime
and workflows / taskProxies / jobs / startedTime
.
ps: Unfortunately I don't see an easy way to summarize the host name per task, so that will be left for later (see previous comments about it too)
Today's progress so far:
<span>
(most common element I think when adding a property on the UI) and use instead a v-layout
with a v-flex
, simulating a Vuetify grid. The icons that are small get the extra attribute shrink
. The other elements that need to expand as much as necessary get the attribute grow
. When necessary, we can also use xs1
or xs2
to represent something akin to 1 or 2 columnsjob
, we know we need to display an icon with the shrink
attribute, then the node name with xs2 md1 lg1
(enough and with plenty of extra space for numbers up to near 1000's, though I doubt we will get tasks with this number of jobs?), and then the hostname with grow
attributecomputed
data structure... but that wouldn't work as the component needs 1 entry item to start iterating and creating the nodes.minDepth
to be displayed. I've seen this in other components, and this may be useful in other cases perhaps.Minimum depth set to 0 (same as before)
**Minimum depth set to 1*** (probably the value used for the Tree
component by default, but for GScan
we may use 0 as we are displaying workflow names there in the sketches).
And etc
And preview on mobile view:
Oh, one more item in the pending list done:
Will need further work later for responsive display, especially when task names are long. But I hope we can move this to another ticket, as the initial version of the Tree
component will have a few features, and preparing a PR with everything done with no issues would take quite a while.
@kinow - I meant to post this late last night, but forgot to actually press the "Comment" button. Not sure if still relevant...
Some task icons are still missing sometimes (a state we don't have yet? ... mentioned above? ...)
Hi @hjoliver
Some task icons are still missing sometimes (a state we don't have yet? ... mentioned above? ...)
I noticed that too: https://github.com/cylc/cylc-ui/issues/189 not quite sure whether it's the data that is wrong, or the Task
component is missing some states?
Ah, thought I had seen it somewhere before (hence my comment "mentioned above?" ... it was late).
I'll comment on #189 ...
The last pending item that I have before preparing a PR, is adding slots so that the component can be customized for both GScan
and Tree
.
When you use a slot, you cannot get a reference for that slot. That makes it harder to keep our current approach with recursion.
What you get instead, is a VNode
, which is a virtual node, used by Vue.js' virtual DOM.
I am hoping if I rewrite the whole template
section as a render
function, then I should be able to pass the slots recursively .... at least in theory, as your are working with virtual nodes in that render
function (I believe the h
is actually the entry point of Vue.js' virtual-dom library). But I will need a few hours (hopefully less than 1 day) to re-write it all for the virtual DOM :disappointed:
Will probably add a few more util methods to collapse all nodes, expand all nodes, and collapse/expand by a filter (e.g. something like collapse( () => node.__type === 'cyclepoint')
, to collapse only cyclepoint nodes, leaving the rest as-is).
Then organize commit history, write some docs about the component options, and prepare 1 or more PR's :guitar:
Nice work @kinow - just tested the latest version :+1:
@kinow - off topic sorry, but I was playing with this branch. IMO div borders should be as thin as possible (or even non-existent if neighbouring divs are different colours) else they look a bit clunky. And as further evidence I don't think I ever see fat borders on other sites these days, e.g. all borders on GitHub seem to be are super-thin.
So instead of this:
Can we do this?
(The diff is a bit too trivial to bother putting up a PR!)
(Probably zero border between side-bar and main page too).
Hi @hjoliver
I think by default Material UI has no borders. We added to match the sketch design. Let me create a ticket for that, and I will see if I can prepare a PR by Friday if there are no objections there :+1:
I like flat design, and would be +1 to remove the borders as well. Looking at the two diffs the second looks better IMO.
And I am having fun creating the component because I spend so long looking at the UI, that it helps me review the layout, find other issues with other components or even with Cylc UI Server. So thanks for remembering to comment on what improvements/issues you are finding for the UI :tada:
@kinow - I meant to post this late last night, but forgot to actually press the "Comment" button. Not sure if still relevant...
Some task icons are still missing sometimes (a state we don't have yet? ... mentioned above? ...)
Probably the "Ready" state.
Added functions in the component to expand and collapse. Example page has controls to "Expand All", "Collapse All", "Expand all Tasks", and "Collapse all Tasks".
The functions are in the component, but the buttons that call them are not. Had to learn how to call methods in a component from outside - you assign a ref
ID to the component, and in Vue.js you call this.$refs.${refId}.method()
.
Also had to work on event propagation. It does seem to have a small performance penalty adding so many events, but the component still looks OK testing with a running suite. A good test will be when we add multiple trees to the user interface.
The expand/collapse feature uses caches (JS Set
). This was based on Vuetify's VTreeView component, which does the same. It means we are not iterating all the nodes, or even going through the DOM. We simply iterate and make sure to keep the Set caches up to date.
"Expand All" and "Collapse All" persist the state. So if you expand all, new nodes will be expanded (default behaviour). If you collapse all, new nodes will be collapsed.
When you pass a filter (in this example we use (treeItem) => treeItem.node.__type == 'task')
, the filter is not persisted, so new nodes will use the value set based on expand/collapse all. It is possible to persist filtered values, but that would add more to the performance, and it's not a requirement for now :+1: (i.e. move this to later :pray: )
Just add slots, maybe try to add some tests, and PR will be ready.
Cheers Bruno
Finished unit tests today. Now adding slots, we just need to confirm visually it looks OK, and the unit tests pass as well.
Did not have to add anything to the project to write the unit tests, just learn how to use Vue Test Utils, especially its Wrapper object, and difference between mount
(mounts the component like real app does) and shallowMount
(mounts the component, but replaces children components by empty stubs - i.e. faster and ignores whatever happens to sub-components).
Unit tests passed on Travis CI without increasing build time noticeable, nor coverage (low as we have included .vue
files now, besides the .js
files that were scanned before).
Just slots and some docs now :sleepy:
A component / style library for displaying nested trees for use by:
Requirements:
Options:
Likely Conclusion
The tree view for Cylc is quite complicated and there likely isn't an off-the-shelf component which can do the job.
Can we borrow code from existing components to give ourselves a head start?