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

Fix: Tag Interface Delete Button #104256

Manually merged
Sybren A. Stüvel merged 39 commits from Evelinealy/flamenco:tag-interface into main 2023-11-02 16:13:14 +01:00
Conversation 3 Commits 39 Files Changed 1 +39 -29
Eveline Anderson commented 2023-09-04 22:55:21 +02:00
Collaborator
Copy Link

Small update to PR #104244

I'm replacing the delete button next to the create tag button and putting it next to the name tags. I'm still working on removing the selecting because I don't want to remove any parts that may be important for the renaming.

Small update to PR #104244 I'm replacing the delete button next to the create tag button and putting it next to the name tags. I'm still working on removing the selecting because I don't want to remove any parts that may be important for the renaming.
Eveline Anderson added 32 commits 2023-09-04 22:55:24 +02:00 
Still needs styling work & maybe some better error checking.
The previous deleteTag() function only removed the tag from the array.
This one actually removes it from the API as well.
# Conflicts:
#	web/app/src/views/TagsView.vue
Eveline Anderson added 1 commit 2023-09-04 23:15:37 +02:00
Eveline Anderson added 1 commit 2023-09-04 23:21:01 +02:00
Eveline Anderson commented 2023-09-04 23:22:15 +02:00
Author
Collaborator
Copy Link

I got it! Removed both the selection ability (safely) and the deletion now works where a user needs to only click the button once. Let me know if there's any bugs that you get. I tested it out and the CRUD operations work on my end.

I got it! Removed both the selection ability (safely) and the deletion now works where a user needs to only click the button once. Let me know if there's any bugs that you get. I tested it out and the CRUD operations work on my end.
Sybren A. Stüvel requested changes 2023-09-04 23:35:35 +02:00
web/app/src/views/TagsView.vue Outdated
@ -141,3 +135,3 @@
const tag_options = {
columns: [
{ title: "Name", field: "name", sorter: "string", editor: "input" },
{
Sybren A. Stüvel commented 2023-09-04 23:34:32 +02:00
Owner
Copy Link

Add this column to the end, so that it has the same layout as the job blocklist (there the ❌ is on the right as well).

Add this column to the end, so that it has the same layout as the job blocklist (there the ❌ is on the right as well).
dr.sybren marked this conversation as resolved
web/app/src/views/TagsView.vue Outdated
@ -147,2 +164,4 @@
sorter: "string",
editor: "input",
vertAlign: "center",
width: 400,
Sybren A. Stüvel commented 2023-09-04 23:35:31 +02:00
Owner
Copy Link

Not sure if a fixed width is going to work well here, especially when the window gets resized.

Not sure if a fixed width is going to work well here, especially when the window gets resized.
dr.sybren marked this conversation as resolved
web/app/src/views/TagsView.vue Outdated
@ -228,2 +247,2 @@
deleteTag() {
if (!this.selectedTag) {
deleteTag(tag) {
if (!tag) {
Sybren A. Stüvel commented 2023-09-04 23:33:12 +02:00
Owner
Copy Link

This shouldn't happen anyway, so you can just remove the entire conditional return here. Better to raise an error that you can see & debug (because of the tag.id access failing) than to silently ignore mistakes.

This shouldn't happen anyway, so you can just remove the entire conditional return here. Better to raise an error that you can see & debug (because of the `tag.id` access failing) than to silently ignore mistakes.
dr.sybren marked this conversation as resolved
web/app/src/views/TagsView.vue Outdated
@ -236,2 +254,3 @@
.deleteWorkerTag(tag.id)
.then(() => {
this.selectedTag = null;
this.tags = this.tags.filter((t) => t.id !== tag.id);
Sybren A. Stüvel commented 2023-09-04 23:32:39 +02:00
Owner
Copy Link

This can be removed. The SocketIO handling will already remove the tag, no matter who did the deletion.

This can be removed. The SocketIO handling will already remove the tag, no matter who did the deletion.
dr.sybren marked this conversation as resolved
Eveline Anderson added 1 commit 2023-09-07 00:27:35 +02:00
Eveline Anderson commented 2023-09-07 00:29:58 +02:00
Author
Collaborator
Copy Link
 this.tabulator.getRows().forEach((row) => {
            if (row.getData().id === tag.id) {
              row.delete();
            }
          });

I'm unsure if this is any better of a solution. I'm trying to work with the SocketIO, but removing some parts is tricky because it shows (obviously) in the backend that there is a removal of the tag, but it will not display on my table. With this solution, the user can see that a tag has been removed from both the frontend table and backend (like when trying to connect a tag to a worker).

``` this.tabulator.getRows().forEach((row) => { if (row.getData().id === tag.id) { row.delete(); } }); ``` I'm unsure if this is any better of a solution. I'm trying to work with the SocketIO, but removing some parts is tricky because it shows (obviously) in the backend that there is a removal of the tag, but it will not display on my table. With this solution, the user can see that a tag has been removed from both the frontend table and backend (like when trying to connect a tag to a worker).
Sybren A. Stüvel commented 2023-09-07 15:18:30 +02:00
Owner
Copy Link
 this.tabulator.getRows().forEach((row) => {
            if (row.getData().id === tag.id) {
              row.delete();
            }
          });

I'm unsure if this is any better of a solution. I'm trying to work with the SocketIO, but removing some parts is tricky because it shows (obviously) in the backend that there is a removal of the tag, but it will not display on my table.

That's for a different PR, then. This PR is for moving the 'delete' button, and because it removes the need for selection of tags, also remove that feature. If there is anything going wrong in the SocketIO signalling, or the handling of that, IMO that should be handled in a separate PR.

> ``` > this.tabulator.getRows().forEach((row) => { > if (row.getData().id === tag.id) { > row.delete(); > } > }); > ``` > I'm unsure if this is any better of a solution. I'm trying to work with the SocketIO, but removing some parts is tricky because it shows (obviously) in the backend that there is a removal of the tag, but it will not display on my table. That's for a different PR, then. This PR is for moving the 'delete' button, and because it removes the need for selection of tags, also remove that feature. If there is anything going wrong in the SocketIO signalling, or the handling of that, IMO that should be handled in a separate PR.
Sybren A. Stüvel added 4 commits 2023-11-02 16:08:24 +01:00 
No functional changes.
- Made the columns in the table scale well
- Moved the 'delete' button to the right-hand side of the table for
  consistency with other tables (like the job blocklist)
- Used the 'red cross' unicode character instead of the letter 'X'
Sybren A. Stüvel approved these changes 2023-11-02 16:09:38 +01:00
Sybren A. Stüvel left a comment
Owner
Copy Link

I've a made a few final changes, so that we can land this PR and include it in v3.3 :)

I've a made a few final changes, so that we can land this PR and include it in v3.3 :)
Sybren A. Stüvel manually merged commit 7ca806f48e into main 2023-11-02 16:13:14 +01:00
Sign in to join this conversation.
Reviewers
No reviewers
Sybren A. Stüvel
Labels
Clear labels   
Good First Issue
  
Priority
High

  
Priority
Low

  
Priority
Normal

  
Status
Archived

  
Status
Confirmed

  
Status
Needs Info from Developers

  
Status
Needs Information from User

  
Status
Needs Triage

  
Status
Resolved

  
Type
Bug

  
Type
Design

  
Type
Job Type
Custom Flamenco job type

  
Type
Known Issue

  
Type
Patch

  
Type
Report

  
Type
To Do

No Label
Good First Issue
Priority
High
Priority
Low
Priority
Normal
Status
Archived
Status
Confirmed
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Job Type
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: studio/flamenco#104256
No description provided.
Delete Branch "Evelinealy/flamenco:tag-interface"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?