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

graph with live data and node info #289

Merged
merged 3 commits into from
Nov 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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,129 changes: 2,011 additions & 2,118 deletions package-lock.json

Large diffs are not rendered by default.

22 changes: 17 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,28 @@
"dependencies": {
"apollo-boost": "^0.4.4",
"axios-fetch": "^1.1.0",
"cytoscape": "^3.9.0",
"cytoscape-cise": "^1.0.0",
"cytoscape-cola": "^2.3.0",
"cytoscape-cose-bilkent": "^4.0.0",
"cytoscape-dagre": "^2.2.2",
"cytoscape-expand-collapse": "^3.1.2",
"cytoscape-navigator": "^1.3.3",
"cytoscape-node-html-label": "^1.1.5",
"cytoscape-panzoom": "^2.5.3",
"cytoscape-popper": "^1.0.4",
"cytoscape-undo-redo": "^1.3.1",
"dotparser": "^0.4.0",
"enumify": "^1.0.4",
"graphql": "^14.5.8",
"graphql-tag": "^2.10.1",
"jquery": "^3.4.1",
"jshint": "^2.10.2",
"nprogress": "^0.2.0",
"tippy.js": "^5.0.2",
"tippy.js": "^4.3.5",
"vee-validate": "^3.0.11",
"vue": "^2.6.10",
"vue-apollo": "^3.0.0-beta.30",
"vue-cytoscape": "^0.2.10",
"vue-i18n": "^8.14.1",
"vue-meta": "^2.3.1",
"vue-router": "^3.1.3",
Expand Down Expand Up @@ -58,9 +69,10 @@
"nyc": "^14.1.1",
"sass": "^1.23.0",
"sass-loader": "^7.3.1",
"sinon": "^7.5.0",
"standard": "^14.3.1",
"vue-cli-plugin-apollo": "^0.21.1",
"sinon": "^7.4.1",
"standard": "^14.0.2",
"svgo": "^1.3.2",
"vue-cli-plugin-apollo": "^0.21.0",
"vue-cli-plugin-eslint-config-vuetify": "latest",
"vue-cli-plugin-vuetify": "latest",
"vue-cli-plugin-vuetify-essentials": "latest",
Expand Down
211 changes: 211 additions & 0 deletions src/components/core/Cytoscape.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,211 @@
<template>
<div id="cytoscape" ref="cytoscape">
<cy-element
v-for="def in elements"
:key="`${def.data.id}`"
:definition="def"
:debug="debug"
></cy-element>
<slot></slot>
Copy link
Member

Choose a reason for hiding this comment

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

These two elements above are confusing @MartinRyan.

I started the review by the leaf component CyElement, which is used by this one (though the cy-element name is not mapped explicitly).

However, I couldn't see my changes, nor attach the debugger/breakpoints. Then I suspected the cytoscape nodes were actually being created on the fly, without the need for the CyElement, and without anything being passed down as value for the default slot of this component.

So looks to me like all that we actually need in the template is <div id="cytoscape" ref="cytoscape"></div>? I did a quick test removing both elements above, and the graph was still being updated with no issues.

</div>
</template>
<script>
import cytoscape from 'cytoscape'
import Vue from 'vue'

export const sync = state => {
Copy link
Member

Choose a reason for hiding this comment

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

I couldn't find this being used anywhere. Can we remove it?

console.log('sync state: ', state)
let elements = [...state.elements]
// replace the `elements` field with a custom one
Object.defineProperty(state, 'elements', {
get () {
return elements
},
set (newElements) {
// update cytoscape view
VueCyObj.instance.then(c => {
// remove all the elements in cytoscape that are not in the newElements
const selector = newElements.map(el => `#${el.data.id}`).join(', ')
c.remove(cy.$(selector).absoluteComplement())
// add new elements if needed
newElements.forEach(el => {
if (c.$(`#${el.data.id}`).length === 0) c.add(el)
})
})
elements = newElements
},
enumerable: true,
configurable: true
})
// trigger the initialization, will add the elements if needed
state.elements = elements
return state
}

let resolver = null
let cy = null

// eslint-disable-next-line no-unused-vars
const promise = new Promise((resolve, reject) => {
resolver = resolve
})

const VueCyObj = {
Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to remove the VueCyObj object @MartinRyan ? And remove the global Vue.prototype.$cytoscape which, as far as I could tell, is not used?

reset () {
cy = null
},
get instance () {
return promise
},
setConfig (config, preConfig, afterCreated) {
// if a pre-configuration function is passed
// then call it with the cytoscape constructor
// this is useful to install/use extensions
if (preConfig) {
preConfig(cytoscape)
}

cy = cytoscape(config)
// if a afterCreated function is passed
// then call it with the cytoscape *instance*
if (afterCreated) {
afterCreated(cy)
}

// let the cytoscape instance available for the awaiters
if (resolver) {
resolver(cy)
}
}
}

Vue.prototype.$cytoscape = VueCyObj

export { VueCyObj }

export default {
props: {
config: {
Copy link
Member

Choose a reason for hiding this comment

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

Config is never passed any value... so it is always null. Same for other props. Looks like the only props ever used are actually preConfig, afterCreated, and debug. Can we remove the others?

<cytoscape id='cytoscape' :pre-config='preConfig' :after-created='afterCreated' :debug='true'>

type: Object,
default: null
},
preConfig: {
type: Function,
default: null
},
afterCreated: {
type: Function,
default: null
},
elementAdded: {
type: Function,
default: null
},
elementRemoved: {
type: Function,
default: null
},
debug: Boolean,
reflection: Boolean
},
data () {
return {
elements: []
Copy link
Member

Choose a reason for hiding this comment

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

This appears to never be used. Can this be removed @MartinRyan ? The cy-element in the template is also never actually used as this is always empty. So both could be removed?

}
},
created () {
if (this.debug) console.log('[Cytoscape] created ')
let els = []
if (this.config && this.config.elements) {
Copy link
Member

Choose a reason for hiding this comment

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

This is never true, as this.config is always null. Can we remove this if statement and the code within it?

if (
this.config.elements.nodes &&
Array.isArray(this.config.elements.nodes)
) {
els = [
...els,
...this.config.elements.nodes.map(n => ({ group: 'nodes', ...n }))
]
}
if (
this.config.elements.edges &&
Array.isArray(this.config.elements.edges)
) {
els = [
...els,
...this.config.elements.edges.map(n => ({
...n,
group: 'edges',
data: {
id: n.data.id ? n.data.id : `${n.data.source}_${n.data.target}`,
...n.data
}
}))
]
}

if (Array.isArray(this.config.elements)) {
els = [...this.config.elements]
}
}

if (this.debug) console.log('[Cytoscape] elements set to', els)
this.elements = els
Copy link
Member

Choose a reason for hiding this comment

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

No effect I think, as this.config is always null?

image

},
async mounted () {
// create a cytoscape instance using the referenced div
const container = this.$refs.cytoscape
const config = {
container, // container to render in
...this.config // the data passed
}
VueCyObj.setConfig(config, this.preConfig, this.afterCreated)
Copy link
Member

Choose a reason for hiding this comment

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

setConfig is only ever called in this method. So perhaps it could be imported from some module like src/components/cylc/graph/index.js or be moved into a function in this component, without the need to exist in VueCyObj (which is also exported in this module) ?

const cy = await VueCyObj.instance
if (this.debug) console.log('[Cytoscape] cy', cy)
// register all the component events as cytoscape ones
for (const [eventType, callback] of Object.entries(this.$listeners)) {
cy.on(eventType, event => callback(event))
}
if (this.reflection) {
Copy link
Member

Choose a reason for hiding this comment

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

It appears that this.reflection is always false, as it has no default value, and the prop is never passed any value by the parent component. Can this if and the code within it be removed?

image

if (this.debug) console.log('[Cytoscape] dom reflection is on')
// listen on elements creation/removal, and perform the reflection here
cy.on('add', event => {
const element = {
group: event.target.group(),
data: event.target.data()
}
if (this.debug) console.log('[Cytoscape] adding element', element)
const { elements } = this
const addReflection = () => {
if (this.debug) console.log('[Cytoscape] [+] added element', element)
elements.push(element)
}
// if there is a hook, then pass the data to it
if (this.elementAdded) this.elementAdded(element, addReflection)
// otherwise, just reflect the changes here
else addReflection()
})
cy.on('remove', event => {
const id = event.target.id()
if (this.debug) console.log('[Cytoscape] removing element', id)
const { elements } = this
const removeReflection = () => {
// console.log('event removing: ', id)
if (this.debug) console.log('[Cytoscape] [x] removed element', id)
// remove the element, in place
elements.splice(elements.findIndex(n => n.data.id === id), 1)
}
// if there is a hook, then pass the data to it
if (this.elementRemoved) this.elementRemoved(id, removeReflection)
// otherwise, just reflect the changes here
else removeReflection()
})
}
}
}
</script>
<style scoped>
#cytoscape {
width: 100%;
height: 100%;
Copy link
Member

Choose a reason for hiding this comment

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

cytoscape-custom.css has #cytoscape { width: 100%, height: 800px }. Shouldn't we define width and height there? Either 100% for both as in this scoped style, or with the values in that stylesheet?

}
</style>
Copy link
Member

Choose a reason for hiding this comment

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

Later I think we can move both Cytoscape.vue and CyElement.vue to cylc/. The core folder on master should have only the files that came with the theme. This might be helpful for the next time we need to update the theme again.

Loading