mutations in the tree view #504
Conversation
| <CylcObject :id="node.node.id"> | ||
| <task | ||
| :status="node.node.state" | ||
| :isHeld="node.node.isHeld" | ||
| :progress=0 | ||
| /> | ||
| </CylcObject> |
There was a problem hiding this comment.
Basic premise:
If you've got something you wan't to be interactive, wrap it with a <CylcObject> and provide it the Cylc ID for the "thing" your element represents.
This wrapper handles context menus, pop-ups and mutations.
I did think about adding this functionality into the task/job components themselves which would make developing new views a little easier, however, this allows groups of components to be wrapped up together.
I'm not 100% happy with this approach. For example, what if you wanted to wrap an entire row in the tree view? You could do it, however, CylcObject and the Tree components would be fighting for interaction events.
Open to ideas.
There was a problem hiding this comment.
Also this really won't work with multiple selection etc. I think for that to work this functionality would have to be much more centralised. Perhaps this is the stepping stone.
There was a problem hiding this comment.
Now thinking that this should be available somehow as a function something along the lines of:
interact([ids], [posX, posY])
This would, for example, allow the Tree view to handle multiple-selection then call the interact function with the IDs of the selected elements and the x,y position of the cursor to create the tooltip at?
In Vue terms this could be:
- A directive, however, I don't know whether a directive can create the context-menu component.
- A mixin for the Cylc "views", however, I don't know whether a mixin can create the context-menu component either.
- A wrapper component that the Cylc "views" are placed inside. The interact method would be passed to the view which would in turn pass it down to whatever components require it.
There was a problem hiding this comment.
Basic premise:
If you've got something you wan't to be interactive, wrap it with a and provide it the Cylc ID for the "thing" your element represents.
This wrapper handles context menus, pop-ups and mutations.
I did think about adding this functionality into the task/job components themselves which would make developing new views a little easier, however, this allows groups of components to be wrapped up together.
Sounds like a good approach, similar to the Vuetify v-skeleton component. It wraps the component(s) and adds it some new feature.
I'm not 100% happy with this approach. For example, what if you wanted to wrap an entire row in the tree view? You could do it, however, CylcObject and the Tree components would be fighting for interaction events.
Hmmm, well thought. But I think you should be able to capture the click or other events in the CylcObject, while allowing the children components to receive the event too, if necessary.
Also this really won't work with multiple selection etc. I think for that to work this functionality would have to be much more centralised. Perhaps this is the stepping stone.
If I understand correctly, the issue would be if we allowed users to select a cycle point and a family proxy at the same time. What would happen when the user click to display the mutations? If so, I think we can just disable the mutations pop up when multiple elements of different types are selected (might be tricky to do that, but doable I think).
In Vue terms this could be:
A directive, however, I don't know whether a directive can create the context-menu component.
Hmm, that would be tricky. Not sure how/where I would start to implement this with a directive. I haven't used any custom directive yet, but from what I've used in other components, it's normally used to enable/disable a feature of a component. So the components (TreeItem for example) would have to have built-in support to mutations, and that directive would just enable/disable it.
I think the current approach keeps a better separation of concerns, but might be wrong.
A mixin for the Cylc "views", however, I don't know whether a mixin can create the context-menu component either.
Yeah, I can't think of a clean way, without making each component aware of the context-menu, at least in the template section we would need to have some if/elses.
A wrapper component that the Cylc "views" are placed inside. The interact method would be passed to the view which would in turn pass it down to whatever components require it.
That could work, but not sure if that'd work with every sub-component. Worth a try if you think that'd be better. At least I think that's the easiest of the three options you suggested.
But I liked the simple CylcObject wrapper 😃
|
Ok, getting close with this, tidied and rebased, need to get started on tests. One blocker is that in the |
If you only need to calculate the values once, I think it would be to use the But if you still want reactivity, you can try with a There are cases when you need to mix |
|
Ok, I've added a state variable, sadly this means it's no longer a functional component but at last it's not performing un-necessary computation on mount. |
| // TODO: fixme | ||
| // expect(taskProxy.$children[0].filtered).to.equal(false) |
There was a problem hiding this comment.
I couldn't work out how to fix these, not sure how to debug. I think this is failing because I've added extra components into the hierarchy?
There was a problem hiding this comment.
That would make sense. Same happened when I added that skeleton loader wrapping components in Lumino widgets.
Codecov Report
@@ Coverage Diff @@
## master #504 +/- ##
==========================================
+ Coverage 66.27% 68.01% +1.73%
==========================================
Files 43 45 +2
Lines 940 1091 +151
Branches 59 59
==========================================
+ Hits 623 742 +119
- Misses 299 331 +32
Partials 18 18
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
oliver-sanders
force-pushed
the
meh
branch
4 times, most recently
from
9002d34
to
84d00d1
Compare
|
Rebased and brought in support for mutating cycle points thanks to #505. |
|
Ok, I think I'll open this now, it needs changes, but it'll take me a fair amount of research to work out how to move forward so perhaps better to do this in stages. This approach could be used in GScan right away but I would suggest holding off as it may well change. |
There was a problem hiding this comment.
Tested functionally, and it worked fine! 👍 Going to look at the code now.
I think I clicked on set-outputs while another task was running, and ended up with:
Which I think means the workflow cannot make any progress 😆 no idea how that happened, but my bad, as five is quite fast, and I was simply clicking on the icons to test and see how the UI reacted 👍
|
|
||
| loadMutations () { | ||
| // TODO: this assumes all workflows use the same schema which is and | ||
| // isn't necessarily true, not quite sure, come back to this later |
|
|
||
| /* | ||
| * Functionality for introspecting mutations, associating them with Cylc | ||
| * objects, filtering and calling mutatons. |
| * true if this field must be provided for the mutation to be called. | ||
| * | ||
| * @param {object} mutation - One Mutation as returned by introspection query. | ||
| * @param {object} types - Types as returned by introspection query. |
There was a problem hiding this comment.
Is it object or Array? Further down we call arg._default = getNullValue(arg.type, types). In getNullValue there's a for (const type of types) {… if an Object is given, it would—I think—fail with Uncaught TypeError: types is not iterable (not sure how to test it from the UI, sorry)
|
The Bootstrap.vue Toast component could serve as reference @oliver-sanders for a different approach for the You create a "toast" with this.$bvToast.toast(`Toast ${this.counter} body content`, {
title: `Toaster ${toaster}`,
toaster: toaster,
solid: true,
appendToast: append
})From any part or your code. That creates a popup with an alert/notification in different parts of the screen. We would just need to make it appear relative to the component that originated the call. The toast component also uses a plug-in. In Vue we can create a plugin that is executed once when the application installs the plug-in, and loads directives, components, creates global singletons, etc. e.g. import CylcObject from '@/components/cylc/cylcObject/CylcObject'
/**
* Cylc Objects plug-in.
*/
export default {
/**
* Called when the Vue application is created, and this plug-in is loaded.
* @param Vue {object} - Vue application
* @param options {*} - options passed to the plug-in (if any)
*/
install (Vue, options) {
// We could...
// register the CylcObject component globally (no need to import)
Vue.component('cylc-object', CylcObject)
// add a global directive
Vue.directive('cylc-object', {
bind (el, binding, vnode, oldVnode) {
// define that a cylc-object will have an @click and automagically
// trigger an event?
}
})
// create a singleton in the application to listen for events
Vue.prototype.$cylcObjectsMenu = {}
}
}Take a look later if that looks like a promising path. If so I can dig a bit more to try to come up with a PR to your PR that
|
|
Sounds promising! |
|
Before trying anything, I rebased, then
On
Nothing major, but having minus one component per |
|
I've got the conflict fixed locally, and also the directive is partly done. Tomorrow will continue, now working on breaking |
…it relative to event source (clientX, clientY)
…d use Object.values/keys/etc)
…This avoids running the computation for every object created, delaying its execution until the user clicks on the mutation.
…in the global menu for mutations)
|
@oliver-sanders CI failed again, sorry. I just checked in the GH UI, and I think I think a merge/rebase accident happened somewhere, and the dependency was removed. Probably copying that dependency and re-running |
There was a problem hiding this comment.
Worked with no issues. I tested with five, and everything appears to have worked fine. JS code looks good! 🚀 thanks @oliver-sanders
|
Sounds like any remaining work is to be down in follow-up PRs, so this is going in 🚀 |
Addresses #339
Add mutations into the tree view.
Caveats:
This approach can be easily incorporated into other views.
TODO (this PR):
Requirements check-list
CONTRIBUTING.mdand added my name as a Code Contributor.