studio/flamenco
studio/flamenco
12
30
Fork 38
Code Issues 63 Pull Requests 5 Releases Wiki Activity

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
Conversation 6 Commits 30 Files Changed 3 +72 -116
1 changed files with 72 additions and 116 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 77a08c4254 - Show all commits

188
web/app/src/views/TagsView.vue
View File

@ -2,11 +2,11 @@
<div class="col col-workers-list">
<h2 class="column-title">Tag Details</h2>
<div class="action-buttons">
<button @click="fetchTags">Refresh</button>
<button @click="deleteTag" :disabled="tags.length === 0">
Delete Tag
</button>
<div class="action-buttons btn-bar-group">
<div class="btn-bar">
<button @click="fetchTags">Refresh</button>
<button @click="deleteTag" :disabled="!selectedTag">Delete Tag</button>
Sybren A. Stüvel commented 2023-09-04 12:35:54 +02:00
Review
Copy Link

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.
</div>
</div>
<div class="action-buttons">
@ -21,103 +21,17 @@
</form>
</div>
<!-- Table to display tags -->
<table v-if="tags.length > 0" class="tag-table">
<thead>
<tr>
<th>Name</th>
<th>Description</th>
</tr>
</thead>
<tbody>
<tr
v-for="tag in tags"
:key="tag.name"
@click="onTagClick(tag)"
:class="{ selected: isSelected(tag) }"
>
<td>{{ tag.name }}</td>
<td>{{ tag.description }}</td>
<!-- Name editing field -->
<td>
<input
type="text"
v-if="isSelected(tag)"
v-model="tag.name"
@blur="updateTagName(tag)"
/>
<span v-else>{{ tag.name }}</span>
</td>
</tr>
</tbody>
</table>
<div v-else class="dl-no-data">
<span>No tags found.</span>
</div>
<div id="tag-table-container"></div>
</div>
<footer class="app-footer"></footer>
</template>
<style scoped>
.action-buttons {
margin-bottom: 10px;
margin-top: 10px;
}
.action-buttons button {
margin-right: 10px;
}
/* Add some basic styling to the table */
.tag-table {
background-color: var(--table-color-background-row-odd);
border-radius: var(--border-radius);
color: var(--color-text-muted);
font-family: var(--font-family-mono);
font-size: var(--font-size-sm);
text-align: left;
width: 100%;
border-collapse: collapse;
margin-top: 20px;
overflow: hidden;
position: relative;
text-align: left;
transform: translateZ(0);
}
.tag-table th {
height: 24px;
white-space: nowrap;
}
.tag-table td {
border: 1px solid #ccc;
padding: 8px;
text-align: left;
}
.tag-table th {
background-color: #f0f0f0;
}
.tag-table tbody tr:hover {
background-color: #f5f5f5;
}
/* Style for selected row */
.tag-table tbody tr.selected {
background-color: rgb(137, 130, 201);
font-weight: bold;
}
.selected {
background-color: #f0f0f0;
}
@import "@/assets/base.css";
Sybren A. Stüvel commented 2023-08-21 09:33:05 +02:00
Outdated
Review
Copy Link

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?
</style>
<script>
import { TabulatorFull as Tabulator } from "tabulator-tables";
import { useWorkers } from "@/stores/workers";
import { useNotifs } from "@/stores/notifications";
import { WorkerMgtApi } from "@/manager-api";
@ -137,22 +51,56 @@ export default {
selectedTag: null,
newTagName: "",
workers: useWorkers(),
activeRowIndex: -1,
Sybren A. Stüvel commented 2023-08-21 09:30:39 +02:00
Outdated
Review
Copy Link

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.
};
},
mounted() {
this.fetchTags();
const vueComponent = this;
const api = new WorkerMgtApi(getAPIClient());
window.api = api;
Sybren A. Stüvel commented 2023-08-21 09:21:52 +02:00
Outdated
Review
Copy Link

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

This line was just for debugging, so it can be removed now.
const tag_options = {
columns: [
{ title: "Name", field: "name", sorter: "string" },
{ title: "Description", field: "description", sorter: "string" },
],
rowFormatter(row) {
const data = row.getData();
const isActive = data.id === vueComponent.activeTagID;
const classList = row.getElement().classList;
classList.toggle("active-row", isActive);
classList.toggle("deletion-requested", !!data.delete_requested_at);
Sybren A. Stüvel commented 2023-08-21 09:22:55 +02:00
Outdated
Review
Copy Link

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.
},
layout: "fitData",
layoutColumnsOnNewData: true,
height: "525px", // Must be set in order for the virtual DOM to function correctly.
selectable: true, // The active worker is tracked by click events, not row selection.
};
this.tabulator = new Tabulator("#tag-table-container", tag_options);
this.tabulator.on("rowClick", this.onRowClick);
this.tabulator.on("tableBuilt", () => {
this.fetchTags();
});
},
methods: {
sortData() {
Sybren A. Stüvel commented 2023-08-21 09:23:53 +02:00
Outdated
Review
Copy Link

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

This function is not called, and thus should be removed.
const tab = this.tabulator;
tab.setSort(tab.getSorters()); // This triggers re-sorting.
},
_onTableBuilt() {
Sybren A. Stüvel commented 2023-08-21 09:24:08 +02:00
Outdated
Review
Copy Link

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

This function is not called, and thus should be removed.
this.fetchTags();
},
fetchTags() {
this.workers
.refreshTags()
.then(() => {
this.tags = this.workers.tags;
this.tabulator.setData(this.tags);
})
.catch((error) => {
const errorMsg = JSON.stringify(error);
@ -168,7 +116,9 @@ export default {
api
.createWorkerTag(newTag)
.then(this.fetchTags)
.then(() => {
this.fetchTags(); // Refresh table data
Sybren A. Stüvel commented 2023-08-21 09:30:03 +02:00
Outdated
Review
Copy Link

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.
})
.catch((error) => {
const errorMsg = JSON.stringify(error);
useNotifs().add(`Error: ${errorMsg}`);
@ -176,34 +126,40 @@ export default {
},
deleteTag() {
if (this.tags.length === 0) {
return;
}
if (!this.selectedTag) {
return;
}
// Find the index of the selected tag in the tags array
const index = this.tags.findIndex(
(tag) => tag.id === this.selectedTag.id
);
if (index !== -1) {
const api = new WorkerMgtApi(getAPIClient());
api
.deleteWorkerTag(this.selectedTag.id)
.then(() => {
const api = new WorkerMgtApi(getAPIClient());
api
.deleteWorkerTag(this.selectedTag.id)
.then(() => {
const index = this.tags.findIndex(
Sybren A. Stüvel commented 2023-08-21 09:27:05 +02:00
Outdated
Review
Copy Link

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.
(tag) => tag.id === this.selectedTag.id
);
if (index !== -1) {
this.tags.splice(index, 1);
this.selectedTag = null;
})
.catch((error) => {
const errorMsg = JSON.stringify(error);
useNotifs().add(`Error: ${errorMsg}`);
});
}
}
this.selectedTag = null;
this.tabulator.setData(this.tags);
})
.catch((error) => {
const errorMsg = JSON.stringify(error);
useNotifs().add(`Error: ${errorMsg}`);
});
},
onTagClick(tag) {
this.selectedTag = tag;
onRowClick(event, row) {
const tag = row.getData();
this.onTagClick(tag, row.getIndex());
},
onTagClick(tag, rowIndex) {
Sybren A. Stüvel commented 2023-08-21 09:29:32 +02:00
Outdated
Review
Copy Link

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.
console.log("Clicked Tag:", tag);
console.log("Selected Tag:", this.selectedTag);
this.selectedTag = this.selectedTag === tag ? null : tag;
Sybren A. Stüvel commented 2023-08-21 09:31:39 +02:00
Outdated
Review
Copy Link

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).
this.activeRowIndex = rowIndex;
},
},