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

Tree Component #145

Closed
oliver-sanders opened this issue Jul 24, 2019 · 51 comments
Closed

Tree Component #145

oliver-sanders opened this issue Jul 24, 2019 · 51 comments

Comments

@oliver-sanders
Copy link
Member

oliver-sanders commented Jul 24, 2019

A component / style library for displaying nested trees for use by:

Requirements:

  1. Nested trees
  2. Arbitrary expansion / collapsing
  3. Selection
    1. Single selection (e.g. the current workflow in GScan)
    2. Multiple selection (click and drag as in the current Tree view)
    3. Multiple non-contiguous selection (e.g. ctrl+click as in the current Tree view)
  4. Activatable (e.g. in GScan the current workflow is "activated", that is highlighted in blue)
  5. Tabulated data representation
    1. Ability to have columns of data (e.g. full tree view)
    2. Column sorting
    3. Show/hide columns
  6. Can operate from the central data store
    • No requirement to reformat data to fit it into the component
  7. Responsive Design
    • Desktop mode: Collapsible nested tree
    • Mobile mode: Paged data with a bread-crumb trail
    • See design document

Options:

Component 1 2 3.1 3.2 3.3 4 5.1 5.2 5.3 6 7
Vuetify Treeview 🆗 🆗 🆗
Vue Tree View 🆗 🆗 🆗

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?

@oliver-sanders
Copy link
Member Author

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

@kinow
Copy link
Member

kinow commented Jul 30, 2019

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 🆗 🆗 🆗
  • 3.1, 3.2, 3.3: not sure if the 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.

@kinow
Copy link
Member

kinow commented Aug 12, 2019

@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.

@hjoliver
Copy link
Member

hjoliver commented Aug 13, 2019

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:

  1. a pure expanding tree view (with collapse/expand of individual task job data at the lowest level)
  2. and a pure (non-tree) table view with super-responsive filtering and sorting

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:

  • use off-the-shelf components for both views (or, relatively small mods)
  • probably better performance for both views
  • less complexity in our own code base

The only(?) disadvantages are:

  • switch to a different view to see the tabular form
  • no family collapse/expand in the tabular form

@cylc/core - any opinions on this?

@kinow
Copy link
Member

kinow commented Aug 13, 2019

@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.

image

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.

@oliver-sanders
Copy link
Member Author

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
Copy link
Member Author

@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!

@kinow
Copy link
Member

kinow commented Aug 13, 2019

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?

@matthewrmshin
Copy link
Contributor

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).

@oliver-sanders
Copy link
Member Author

On the multi-selection

I think we at least need to implement the same multi-selection functionality we have in the Cylc7 GUI:

  • Selecting a family selects all tasks.
  • Selecting a range including families and tasks possible.
  • CTRL+Click allows you to select non-adjacent tasks.

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.

@kinow
Copy link
Member

kinow commented Aug 13, 2019

Good points, noted! Multi selection across workflows it will be then. Thanks!

@kinow
Copy link
Member

kinow commented Aug 14, 2019

GIFrecord_2019-08-14_124900

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 ? '&#9661;' : '&#9655;' }}</span>
      <span class="type" v-else>&#9671;</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.

image

@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 :) )

@hjoliver
Copy link
Member

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.

@kinow
Copy link
Member

kinow commented Aug 14, 2019

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:

GIFrecord_2019-08-14_143758

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:
    • instead of keeping cycle points in a Set and iterating over for each workflow, it is using a Map
    • instead of using the 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 recursively

So every entry in the Tree is a TreeItem component instance, with depth, expanded flag, and the node.

Untitled

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 😞

@kinow
Copy link
Member

kinow commented Aug 14, 2019

Oh, and the reason for adding slots, is so that this component can be used in both Tree.vue and GScan.vue.

@kinow
Copy link
Member

kinow commented Aug 14, 2019

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 👍

@hjoliver
Copy link
Member

Looking good!

@oliver-sanders
Copy link
Member Author

Wow @kinow that was quick. Your approach looks sensible to me, nice and flexible. 👏

@matthewrmshin
Copy link
Contributor

@kinow Just took a look at your branch. Big result with so little logic!

@kinow
Copy link
Member

kinow commented Aug 14, 2019

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 👍

@sadielbartholomew
Copy link
Contributor

I can't take much credit.

Please take some much-deserved credit Bruno! This looks great 💯

@kinow
Copy link
Member

kinow commented Aug 15, 2019

Added features to

  • hover
  • mark as active on click (can be done programmatically as well, like for navigation)
  • mark as active on click with multiple values allowed

The current implementation is based on the Vuetify TreeView component. Gif showing these features:

GIFrecord_2019-08-15_151629

Some questions after looking at the current result and the sketch:

1. What icon should we have for Workflow and Cyclepoint?

  • 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

image

2. What should be the order of jobs?

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?

image

3. What is the text next to the Task summary and how to select its value?

I think that's the Job host?

image

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?

4. How should we handle multi-select?

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...

5. Align jobs the same way for GScan and Tree?

The Tree component appears to align job icons to the left when summarizing a Task:

image

While the GScan component seems to align job icons to the right when summarizing a Workflow:

image

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?

@kinow
Copy link
Member

kinow commented Aug 15, 2019

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

@hjoliver
Copy link
Member

hjoliver commented Aug 15, 2019

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

  1. (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?)

@hjoliver
Copy link
Member

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.

@oliver-sanders
Copy link
Member Author

oliver-sanders commented Aug 15, 2019

To further elaborate on Hillary's comments.

  1. What icon should we have for Workflow and Cyclepoint?

Workflow:

  • A suite status icon (play, pause, stop).
  • Note: I'm not sure we will implement viewing multiple suites in one view for early Cylc8.
  • Note: we may want to hide the workflow if only one workflow is selected.

Cyclepoint:

  • Task status icon

what is the formula for setting the Task state in the cyclepoint

  • In Cylc7 we use the most significant task/job status (e.g. if one job in a cycle is running the cycle is marked as running)
  • See this logic
    • PS: For bonus marks we could potentially request the enumerations of (ordered) task/job states from the suite.
    • PPS: I have a branch looking at formally enumerating task/job states.

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.

  1. What should be the order of jobs?

Most recent job at the top

  1. 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:

  1. The most recent job (might not work well in the future where the 1-1 task-job mapping is broken)
  2. Only collapse jobs when there is only one job.
  1. How should we handle multi-select?
  • I think there will be demand for either shift+click and or drag-select for quickly selecting groups of tasks.

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.

  1. Align jobs the same way for GScan and Tree?
  • Right-align for jobs makes sense in GScan.
  • For the Tree view due to long task names, large numbers of tasks and smaller font-size I think it might be hard to visually associate jobs with tasks if we right-aligned jobs here too which is why they are left-aligned in plans.

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.

@matthewrmshin
Copy link
Contributor

If we have room to display a message for a job, I'll vote for the job host/platform + the job ID.

@hjoliver
Copy link
Member

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?

@oliver-sanders
Copy link
Member Author

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).

@kinow
Copy link
Member

kinow commented Aug 20, 2019

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 👍

I like flat design, and would be +1 to remove the borders as well. Looking at the two diffs the second looks better IMO.

@kinow
Copy link
Member

kinow commented Aug 20, 2019

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 🎉

@oliver-sanders
Copy link
Member Author

@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.

@kinow
Copy link
Member

kinow commented Aug 20, 2019

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.

GIFrecord_2019-08-21_110354

"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 👍 (i.e. move this to later 🙏 )

Just add slots, maybe try to add some tests, and PR will be ready.

Cheers
Bruno

@kinow
Copy link
Member

kinow commented Aug 22, 2019

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 😪

@kinow kinow mentioned this issue Aug 23, 2019
@kinow kinow added this to the 0.1 milestone Sep 10, 2019
@kinow kinow mentioned this issue Sep 10, 2019
@kinow kinow modified the milestones: 0.1, 0.2 Sep 14, 2019
@kinow kinow modified the milestones: 0.2, 1.0 Oct 11, 2019
@kinow kinow mentioned this issue Nov 4, 2019
9 tasks
@kinow kinow modified the milestones: 1.0, 2.0 Sep 10, 2021
@kinow kinow removed their assignment Oct 6, 2021
@oliver-sanders oliver-sanders modified the milestones: 2.0.0, Pending Jun 8, 2022
@oliver-sanders oliver-sanders removed this from the Pending milestone Oct 6, 2022
@oliver-sanders
Copy link
Member Author

Closing this as addressed by the many PRs to date. The remaining work is covered by #434.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants