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

Infinite tree #345

Closed
wants to merge 50 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
c91aba3
Add infinite-tree dependency
Dec 19, 2019
ac1bfd3
Add vue-virtual-scroller dependency
kinow Dec 21, 2019
4f1aba0
Update dependencies
kinow Dec 23, 2019
1ec5ce3
Create InfiniteTree component, add styles, and use a RecycleScroller too
Dec 20, 2019
2528441
Move tree components under the tree folder/package
kinow Dec 21, 2019
d8e2bfc
Update new components to match the old Tree/TreeItem component styles…
kinow Dec 21, 2019
6e7fcd7
Use relative import to workaround issue with vue-cli/vue-loader and d…
kinow Dec 23, 2019
21f90ce
Tweak width and margins (updated vuetify)
kinow Dec 23, 2019
6f003fd
Ignore console.debug in workflow service
kinow Dec 23, 2019
15b496e
Remove old TreeItem component and (now unused) event bus
kinow Dec 26, 2019
8fea73a
Disable old tests
kinow Dec 26, 2019
438df4d
Delete old test for treeitem (component was removed, InfiniteTree wil…
kinow Dec 26, 2019
44f03b7
Remove old tests for Tree for features that are not implemented for t…
kinow Dec 27, 2019
61c3bf9
Node need to watch the treeData with deep: true
kinow Dec 27, 2019
303e640
Keep the array of widget IDs in sync
Jan 19, 2020
a75b89f
Make sure event listeners are removed to avoid memory leaks
Jan 19, 2020
d0c83e4
Undo changes to dependencies that are not directly related to the Inf…
Jan 19, 2020
be2487d
Temporarily accept github.com npm files due to missing release of inf…
Jan 19, 2020
edf543e
Remove duplicate HTML id attribute main, and instead use a main-wrapp…
Jan 29, 2020
a9bc2c7
Add the component $el, instead of one of its children nodes.
Jan 29, 2020
8e7e0f2
remove unused events and data property "node"
Jan 29, 2020
2dc994f
Remove unused class attribute
Jan 29, 2020
2a23013
Emit events only if the component was created with the event handlers…
Jan 29, 2020
6d96b22
Add c-tree class so that leaf nodes are rendered correctly
Jan 29, 2020
5fadbc7
v-col must go under v-row
Jan 29, 2020
0f52a62
Reset the listeners of the vue-virtual-scroller component.
Jan 30, 2020
09c64d7
Remove duplicate code in Tree view
Feb 13, 2020
c6a7a41
Use cyclepoints from query instead of looking for it in the family pr…
Feb 13, 2020
a1f61e9
Avoid duplicated text queries
Feb 13, 2020
0a41799
Remove treeitem (not used with infinite tree, left-over after rebase)
Feb 20, 2020
b743743
Apply recent changes (new code to avoid data duplication) to the infi…
Feb 20, 2020
0dce2a9
Fix unit tests
Feb 20, 2020
c962145
Fix Toolbar, to display Graph, Mutation, Tree, etc
Feb 20, 2020
5c1d6f5
Re-apply div+flex styles fixed in (now removed) TreeItem
Feb 20, 2020
7d17009
Adjust dependencies due to eslint and babel issues
kinow Aug 4, 2020
60d388f
Fix code lost during merge-rebase, and also fix bug due to vuetify sk…
kinow Aug 4, 2020
48f6c06
Fix linter command
kinow Aug 4, 2020
e67d94f
Update treeitem children elements.
kinow Aug 4, 2020
a91b872
Fix Tree View for the infinite tree (props, and paddings)
kinow Aug 4, 2020
8ef7342
Update unit tests for the infinite tree
kinow Aug 5, 2020
5e5b178
Dynamically set the number of properties
kinow Aug 5, 2020
c10f00f
Remove unnecessary extra element (less nested elements -> better perf…
kinow Aug 5, 2020
9b9c42b
Use only the watcher to initialize the InfiniteTree.
kinow Aug 5, 2020
2b5ebcd
Remove unused files
kinow Aug 5, 2020
13d6c6f
Show expand/collapse buttons based on CSS classes instead of using JS
kinow Aug 10, 2020
2104535
Avoid redeclaring SASS variables (merging issue?)
kinow Aug 10, 2020
49d3e6a
Remove updated graph dependencies
kinow Aug 10, 2020
787f17f
Remove properties that were used in the React example, but are NOT us…
kinow Aug 11, 2020
dcef4ee
JSDoc: say that the height is in pixels
kinow Aug 11, 2020
7a383d2
Handle deltas subscriptions according to the Vue route selected.
kinow Aug 11, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@ jobs:
- name: Install
run: yarn install
- name: Lint
# TODO: lockfile-lint accepts GitHub for now, as we need infinite-tree from a commit (due to an existing issue in latest release)
# TODO: had to remove --validate-https due to GitHub issues when downloading the infinite-tree; re-enable after their release
run: |
yarn run lint --no-fix
DEBUG="lockfile-lint,validate-host-manager" npx lockfile-lint --path yarn.lock --validate-https --allowed-hosts npm --allowed-hosts registry.yarnpkg.com
DEBUG="lockfile-lint,validate-host-manager" npx lockfile-lint --path yarn.lock --allowed-hosts npm --allowed-hosts registry.yarnpkg.com github.com
- name: Test
run: |
yarn run coverage:unit
Expand Down
21 changes: 11 additions & 10 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
"@lumino/default-theme": "^0.1.8",
"@lumino/widgets": "^1.9.3",
"apollo-boost": "^0.4.4",
"apollo-utilities": "^1.3.2",
"axios-fetch": "^1.1.0",
"cytoscape": "^3.9.0",
"cytoscape-dagre": "^2.2.2",
Expand All @@ -37,28 +36,30 @@
"graphiql": "^0.17.5",
"graphql": "^14.5.8",
"graphql-tag": "^2.10.1",
"infinite-tree": "git+https://[email protected]/cheton/infinite-tree.git#7bf037ee79b8aa702f5ee605b8ec218bab2533b4",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we not using the released version?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I thought I had left a comment somewhere... I clearly remember adding a comment above this line, but I had thought I was writing YAML and of course yarn install failed.

I had to change the GitHub action linter to allow github.com too, but I only mention the issue in the comment there. Let me chase and see what was the issue.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, here it's.

I've downgraded to the latest 1.16.2 release from NPM.

image

I recall now spending a long time staring at the screen, not understanding why it wouldn't work. After trying every possible setting, and reading the library code, I decided to check if there were un-released commits.

As there were some, I pointed to the latest commit, and everything worked. I cannot tell which commit fixes it, but the next release should include it. And I haven't found any other issues using this commit, so far.

Should we create a follow-up issue set for 0.3, saying why we have that pinned commit in the package.json, and that prior to a release of Cylc UI, we should check if there's a version newer than 1.16.2, and if so, what steps we must take (e.g. update version in package.json, remove github.com entry from linter)?

"jshint": "^2.10.2",
"nprogress": "^0.2.0",
"react": "^15.6.2",
"react-dom": "^15.6.2",
"tippy.js": "^4.3.5",
"vue": "^2.6.10",
"vue-i18n": "^8.14.1",
"vue": "^2.6.11",
"vue-i18n": "^8.15.3",
"vue-markdown": "^2.2.4",
"vue-meta": "^2.3.1",
"vue-router": "^3.1.3",
"vue-spinner": "^1.0.3",
"vue-the-mask": "^0.11.1",
"vue-virtual-scroller": "^1.0.0-rc.2",
"vuetify": "^2.2.6",
"vuex": "^3.1.1",
"vuex": "^3.1.2",
"vuex-router-sync": "^5.0.0"
},
"devDependencies": {
"@babel/plugin-proposal-class-properties": "^7.8.3",
"@cypress/code-coverage": "^2.0.5",
"@cypress/webpack-preprocessor": "^5.1.2",
"@mdi/font": "^4.5.95",
"@vue/cli-plugin-babel": "^3.12.0",
"@vue/cli-plugin-babel": "^4.1.1",
"@vue/cli-plugin-e2e-cypress": "^4.3.1",
"@vue/cli-plugin-eslint": "^4.2.3",
"@vue/cli-plugin-unit-mocha": "^4.2.3",
Expand All @@ -74,7 +75,7 @@
"chai": "^4.2.0",
"cross-env": "^6.0.3",
"cross-fetch": "^3.0.4",
"eslint": "^7.0.0",
"eslint": "^6.5.1",
"eslint-config-standard": "^14.1.1",
"eslint-config-vuetify": "^0.5.0",
"eslint-plugin-node": "^11.1.0",
Expand All @@ -91,13 +92,13 @@
"standard": "^14.3.1",
"subscriptions-transport-ws": "^0.9.16",
"svgo": "^1.3.2",
"vue-cli-plugin-apollo": "^0.21.0",
"vue-cli-plugin-apollo": "^0.21.3",
"vue-cli-plugin-eslint-config-vuetify": "0.0.3",
"vue-cli-plugin-vuetify": "^2.0.5",
"vue-cli-plugin-vuetify-essentials": "^0.8.3",
"vue-template-compiler": "^2.6.10",
"vuetify-loader": "^1.3.0",
"webpack": "^4.41.0"
"vue-template-compiler": "^2.6.11",
"vuetify-loader": "^1.4.3",
"webpack": "^4.41.4"
},
"resolutions": {
"istanbul-instrumenter-loader/**/istanbul-lib-instrument": "4.0.1"
Expand Down
2 changes: 0 additions & 2 deletions src/components/cylc/Toolbar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
>
<v-list-item-title><v-icon>mdi-file-tree</v-icon> Tree</v-list-item-title>
</v-list-item>
</v-list>
<v-list class="pa-0">
<v-list-item
class="py-0 px-8 ma-0"
@click="onClickAddGraphView"
Expand Down
186 changes: 186 additions & 0 deletions src/components/cylc/tree/InfiniteTree.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
<template>
<!-- NB: we are setting the itemSize to null to use the "Variable size mode" of the vue-virtual-scroller -->
<RecycleScroller
page-mode
ref="scroller"
key-field="id"
:items="tree.nodes"
:item-size="null"
>
<!-- eslint-disable-next-line vue/no-unused-vars -->
<div slot-scope="{ item, index }">
<slot
name="default"
v-bind="{
node: item,
tree: tree
}"
/>
</div>
</RecycleScroller>
</template>

<script>
import { RecycleScroller } from 'vue-virtual-scroller'
// TODO: import InifiniteTree from 'infinite-tree' is not working with dependency installed from git commit. Replace once there is a new release of infinite-tree
import InfiniteTree from '../../../../node_modules/infinite-tree/dist/infinite-tree'

const lcfirst = (str) => {
str += ''
return str.charAt(0).toLowerCase() + str.substr(1)
}
oliver-sanders marked this conversation as resolved.
Show resolved Hide resolved

export default {
name: 'InfiniteTree',
components: {
RecycleScroller
},
props: {
treeData: {
type: [Array, Object],
default: () => {
return []
}
},
autoOpen: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought for later work, for default expansion/collapsing we could use:

  • A maximum expand depth
    • Which is what we currently do with Cylc7 [and in my Cylc Monitor PR] (i.e. expand the first N levels)*
  • A function or list of conditionals.
    • Perhaps slower but very powerful, would allow us to "filter" the display, fits into the infinite tree model well.
    • e.g. collapse parameterisations by default.
    • If nodes know their depth we could implement "maximum expand depth" in this way.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before that would be a bit harder, as the collapse/extend state was controlled in the component.

The InifiniteTree object keeps state in the data. So we have full control by simply choosing the initial/default state and setting the value in the JavaScript data.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If nodes know their depth we could implement "maximum expand depth" in this way.

Nodes do know their depth 😉

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True! But I should have explained better. I think I know one case that doesn't translate directly from the GraphQL results to the Tree view.

I don't remember how we are using depth (i.e. don't remember which nodes in our schema have it). But say we have a workflow with depth=0, then the next step would be cycle points with depth=1 in the tree.

But in our schema, I think the cycle point has a different depth?

We also have special cases, like families. I am not sure if the depth in families is using the correct hierarchy that we need for the tree—which is good I think; our GraphQL schema is independent of component.

Another corner case is the root family, which is hidden by default. So the depth has to be corrected for that. So in the end, we need the depth matching what we want to display in the Tree component, which is too specific for the component I think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember how we are using depth (i.e. don't remember which nodes in our schema have it). But say we have a workflow with depth=0, then the next step would be cycle points with depth=1 in the tree.

Depth is defined on all nodes except jobs, and based on Family hierarchy.. Cycle points in the tree view are really the root family in my mind... (BTW - maxDepth is a field under workflow if it helps with working out minimum margins or something)

type: Boolean,
default: false
},
selectable: {
type: Boolean,
default: false
},
loadNodes: {
type: Function,
default: () => {}
},
shouldSelectNode: {
type: Function,
default: () => {}
},
shouldLoadNodes: {
type: Function,
default: () => {}
},
// Callback invoked before updating the tree.
onContentWillUpdate: {
type: Function,
default: null
},
// Callback invoked when the tree is updated.
onContentDidUpdate: {
type: Function,
default: null
},
// Callback invoked when a node is opened.
onOpenNode: {
type: Function,
default: null
},
// Callback invoked when a node is closed.
onCloseNode: {
type: Function,
default: null
},
// Callback invoked when a node is selected or deselected.
onSelectNode: {
type: Function,
default: null
},
// Callback invoked before opening a node.
onWillOpenNode: {
type: Function,
default: null
},
// Callback invoked before closing a node.
onWillCloseNode: {
type: Function,
default: null
},
// Callback invoked before selecting or deselecting a node.
onWillSelectNode: {
type: Function,
default: null
},
onKeyUp: {
type: Function,
default: null
},
onKeyDown: {
type: Function,
default: null
},
onMouseLeave: {
type: Function,
default: null
},
onMouseEnter: {
type: Function,
default: null
}
},
inheritAttrs: false,
data () {
return {
tree: new InfiniteTree({
el: this.$refs.tree,
...this.$props
}),
nodes: [],
eventHandlers: {
onContentWillUpdate: null,
onContentDidUpdate: null,
onOpenNode: null,
onCloseNode: null,
onSelectNode: null,
onWillOpenNode: null,
onWillCloseNode: null,
onWillSelectNode: null,
onKeyUp: null,
onKeyDown: null,
onMouseEnter: null,
onMouseLeave: null
}
}
},
watch: {
treeData: {
handler (newValue) {
this.tree.loadData(newValue)
this.nodes = this.tree.nodes
},
immediate: true
}
},
mounted () {
// set up event handlers, if any provided
Object.keys(this.eventHandlers).forEach(key => {
if (!this[key]) {
return
}
const eventName = lcfirst(key.substr(2)) // e.g. onContentWillUpdate -> contentWillUpdate
this.eventHandlers[key] = this[key]
this.tree.on(eventName, this.eventHandlers[key])
})
},
beforeDestroy () {
Object.keys(this.eventHandlers).forEach(key => {
if (!this.eventHandlers[key]) {
return
}
const eventName = lcfirst(key.substr(2)) // e.g. onUpdate -> update
this.tree.removeListener(eventName, this.eventHandlers[key])
this.eventHandlers[key] = null
})

this.tree.destroy()
this.tree = null
},
methods: {
resetListeners () {
this.$refs.scroller.removeListeners()
this.$refs.scroller.addListeners()
}
}
}
</script>
Loading