Web Interface for Tags #104244

Merged
Sybren A. Stüvel merged 30 commits from Evelinealy/flamenco:tag-interface into main 2023-09-04 13:06:10 +02:00
Collaborator

Fix #104204: Web interface for CRUD operations of worker tags.

There are still a few things I want to change/add before taking this off the WIP list:

  • Adding proper classes to Tabulator for styling
  • Allowing the user to write a description
  • Preventing long text description from going off the page
  • Implementing update (limited information in this in Tabulator's documentation) to be enabled when pushing the "update" button
  • Input create button inline with new tag input form
Fix #104204: Web interface for CRUD operations of worker tags. There are still a few things I want to change/add before taking this off the WIP list: - [x] Adding proper classes to Tabulator for styling - [x] Allowing the user to write a description - [x] Preventing long text description from going off the page - [x] Implementing update (limited information in this in Tabulator's documentation) to be enabled when pushing the "update" button - [x] Input create button inline with new tag input form
Eveline Anderson added 18 commits 2023-08-18 23:30:55 +02:00
Sybren A. Stüvel reviewed 2023-08-21 09:33:35 +02:00
Sybren A. Stüvel left a comment
Owner

It's working! I've left a bunch of inline comments.

It's working! I've left a bunch of inline comments.
@ -0,0 +27,4 @@
</template>
<style scoped>
@import "@/assets/base.css";

There shouldn't be any need to import this CSS file here. Which problem is this solving?

There shouldn't be any need to import this CSS file here. Which problem is this solving?
@ -0,0 +51,4 @@
selectedTag: null,
newTagName: "",
workers: useWorkers(),
activeRowIndex: -1,

activeRowIndex is only set, but never read. That means it can be removed.

`activeRowIndex` is only set, but never read. That means it can be removed.
@ -0,0 +60,4 @@
const vueComponent = this;
const api = new WorkerMgtApi(getAPIClient());
window.api = api;

This line was just for debugging, so it can be removed now.

This line was just for debugging, so it can be removed now.
@ -0,0 +72,4 @@
const isActive = data.id === vueComponent.activeTagID;
const classList = row.getElement().classList;
classList.toggle("active-row", isActive);
classList.toggle("deletion-requested", !!data.delete_requested_at);

I think this is a copy-paste leftover of the jobs table; tags are immediately deleted, whereas jobs are first marked as 'deletion requested' and then deleted by a background process. This line can be removed.

I think this is a copy-paste leftover of the jobs table; tags are immediately deleted, whereas jobs are first marked as 'deletion requested' and then deleted by a background process. This line can be removed.
@ -0,0 +88,4 @@
},
methods: {
sortData() {

This function is not called, and thus should be removed.

This function is not called, and thus should be removed.
@ -0,0 +92,4 @@
const tab = this.tabulator;
tab.setSort(tab.getSorters()); // This triggers re-sorting.
},
_onTableBuilt() {

This function is not called, and thus should be removed.

This function is not called, and thus should be removed.
@ -0,0 +117,4 @@
api
.createWorkerTag(newTag)
.then(() => {
this.fetchTags(); // Refresh table data

I think it's also nice to clear the input field once the tag has been succesfully created.

I think it's also nice to clear the input field once the tag has been succesfully created.
@ -0,0 +134,4 @@
api
.deleteWorkerTag(this.selectedTag.id)
.then(() => {
const index = this.tags.findIndex(

Instead of trying to find the just-deleted tag, just call this.fetchTags().

This is all temporary code, and that it should be removed once the SocketIO broadcasting of tag changes is implemented.

Instead of trying to find the just-deleted tag, just call `this.fetchTags()`. This is all temporary code, and that it should be removed once the SocketIO broadcasting of tag changes is implemented.
@ -0,0 +155,4 @@
this.onTagClick(tag, row.getIndex());
},
onTagClick(tag, rowIndex) {

Not sure why this is implemented in a separate function, it could just all be part of the onRowClick event handler.

Not sure why this is implemented in a separate function, it could just all be part of the `onRowClick` event handler.
@ -0,0 +158,4 @@
onTagClick(tag, rowIndex) {
console.log("Clicked Tag:", tag);
console.log("Selected Tag:", this.selectedTag);
this.selectedTag = this.selectedTag === tag ? null : tag;

This alone is not enough to update all the rows in the table. When I click one tag, and then another, both appear as if they're selected. You probably need to force a redraw of the table somehow (there's a Tabulator API call for this).

This alone is not enough to update all the rows in the table. When I click one tag, and then another, both appear as if they're selected. You probably need to force a redraw of the table somehow (there's a Tabulator API call for this).
@ -0,0 +165,4 @@
computed: {
isSelected() {
return (tag) => tag === this.selectedTag;

isSelected is not used, so it can be removed. Also I don't see it working, given that it uses an undefined variable tag.

`isSelected` is not used, so it can be removed. Also I don't see it working, given that it uses an undefined variable `tag`.
Eveline Anderson added 1 commit 2023-08-21 22:29:26 +02:00
Eveline Anderson added 1 commit 2023-08-21 23:31:02 +02:00
Author
Collaborator

I'm still struggling with the process of allowing the edited tag names to show up in the API. IE. In the columns, all I did was adding the editor:string to the tag:
{ title: "Name", field: "name", sorter: "string", editor: "input" },

The tricky part is getting the API to see it. I know I need to use

table.on("cellEdited", function(cell){
        //cell - cell component
});

from here: https://www.tabulator.info/docs/5.5/events#edit

I'll figure it out, but it'll take me a moment. I've committed it, but haven't pushed just yet. Good news too, Tabulator automatically removes any overlaying text with "..." so I don't need to worry about that. The main thing is the update tag, and then removing all the redundant code I had in there that you pointed out.

I'm still struggling with the process of allowing the edited tag names to show up in the API. IE. In the columns, all I did was adding the editor:string to the tag: `{ title: "Name", field: "name", sorter: "string", editor: "input" },` The tricky part is getting the API to see it. I know I need to use ``` table.on("cellEdited", function(cell){ //cell - cell component }); ``` from here: https://www.tabulator.info/docs/5.5/events#edit I'll figure it out, but it'll take me a moment. I've committed it, but haven't pushed just yet. Good news too, Tabulator automatically removes any overlaying text with "..." so I don't need to worry about that. The main thing is the update tag, and then removing all the redundant code I had in there that you pointed out.

I'm still struggling with the process of allowing the edited tag names to show up in the API

What does that mean?

. IE. In the columns, all I did was adding the editor:string to the tag:
{ title: "Name", field: "name", sorter: "string", editor: "input" },

The tricky part is getting the API to see it.

Which API? Tabulator API? Flamenco API?

I know I need to use

table.on("cellEdited", function(cell){
        //cell - cell component
});

from here: https://www.tabulator.info/docs/5.5/events#edit

Be sure to read the right version of the docs. Flamenco is using Tabulator 5.4 at the moment.

> I'm still struggling with the process of allowing the edited tag names to show up in the API What does that mean? >. IE. In the columns, all I did was adding the editor:string to the tag: > `{ title: "Name", field: "name", sorter: "string", editor: "input" },` > > The tricky part is getting the API to see it. Which API? Tabulator API? Flamenco API? > I know I need to use > ``` > table.on("cellEdited", function(cell){ > //cell - cell component > }); > ``` > from here: https://www.tabulator.info/docs/5.5/events#edit Be sure to read the right version of the docs. Flamenco is using Tabulator 5.4 at the moment.
Author
Collaborator

Thank you for the heads up, I'll switch back to 5.4's version. And it's the Worker Manager API. For example, when you delete a tag, that tag is also removed as an option when trying to pick a tag to assign to a worker. I'm in the process of doing the same thing for the updating of a tag on the table.

Thank you for the heads up, I'll switch back to 5.4's version. And it's the Worker Manager API. For example, when you delete a tag, that tag is also removed as an option when trying to pick a tag to assign to a worker. I'm in the process of doing the same thing for the updating of a tag on the table.
Eveline Anderson added 2 commits 2023-08-22 22:29:03 +02:00
Eveline Anderson added 1 commit 2023-08-22 22:44:40 +02:00
Eveline Anderson added 3 commits 2023-08-22 22:57:38 +02:00
Eveline Anderson changed title from WIP: Web Interface for Tags to Web Interface for Tags 2023-08-22 23:01:29 +02:00
Eveline Anderson added 1 commit 2023-08-22 23:11:42 +02:00
Eveline Anderson added 1 commit 2023-08-22 23:17:39 +02:00
Author
Collaborator

UPDATE:

I've finished up most of the "major" tasks and removed some of the redundant code (yay)! So the table should now be able to update, delete, and add tags successfully. What else am I missing from the interface?

UPDATE: I've finished up most of the "major" tasks and removed some of the redundant code (yay)! So the table should now be able to update, delete, and add tags successfully. What else am I missing from the interface?

Thank you for the heads up, I'll switch back to 5.4's version. And it's the Worker Manager API. For example, when you delete a tag, that tag is also removed as an option when trying to pick a tag to assign to a worker. I'm in the process of doing the same thing for the updating of a tag on the table.

Ah, that's the SocketIO broadcasting. I wouldn't do that as part of this PR. I'll take care of that after this PR lands.

> Thank you for the heads up, I'll switch back to 5.4's version. And it's the Worker Manager API. For example, when you delete a tag, that tag is also removed as an option when trying to pick a tag to assign to a worker. I'm in the process of doing the same thing for the updating of a tag on the table. Ah, that's the SocketIO broadcasting. I wouldn't do that as part of this PR. I'll take care of that after this PR lands.
Eveline Anderson added 1 commit 2023-08-24 22:35:37 +02:00
Author
Collaborator

Got your message on Blender Chat too. I'm taking a look at it now and going through the changes you made. Thanks for handling the SocketIOs!

Got your message on Blender Chat too. I'm taking a look at it now and going through the changes you made. Thanks for handling the SocketIOs!
Eveline Anderson added 1 commit 2023-08-24 23:02:09 +02:00
Sybren A. Stüvel approved these changes 2023-09-04 13:04:00 +02:00
Sybren A. Stüvel left a comment
Owner

Nice work!

Nice work!
@ -0,0 +5,4 @@
<div class="action-buttons btn-bar-group">
<div class="btn-bar">
<button @click="fetchTags">Refresh</button>
<button @click="deleteTag" :disabled="!selectedTag">Delete Tag</button>

For a followup PR, I think it would be nicer to remove this button, and have a little icon behind each tag (just like in Blocklist.vue for removing blocklist entries).

Since selection of tags is only used for deleting them, this little change would remove the entire need to select tags. It also clears up the double semantics of clicking on a tag name: it currently both selects the tag and allows renaming it.

For a followup PR, I think it would be nicer to remove this button, and have a little ❌ icon behind each tag (just like in `Blocklist.vue` for removing blocklist entries). Since selection of tags is only used for deleting them, this little change would remove the entire need to select tags. It also clears up the double semantics of clicking on a tag name: it currently both selects the tag and allows renaming it.
Sybren A. Stüvel merged commit c68a72ca49 into main 2023-09-04 13:06:10 +02:00
Sybren A. Stüvel deleted branch tag-interface 2023-09-04 13:06:11 +02:00
Sign in to join this conversation.
No description provided.