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

Add option to delete stored servers from combo lists #1421

Closed
pgScorpio opened this issue Apr 1, 2021 · 26 comments · Fixed by #3159
Closed

Add option to delete stored servers from combo lists #1421

pgScorpio opened this issue Apr 1, 2021 · 26 comments · Fixed by #3159
Assignees
Labels
feature request Feature request

Comments

@pgScorpio
Copy link
Contributor

Has this feature been discussed and generally agreed?

Issue has been mentioned multiple times in discussion groups and on Jamulus facebook groups, but afaik not on github.
The given solution is always to edit the ini file (or even to delete it!), but I don't think this is very user friendly.

Describe the solution you'd like

Add an option to delete servers from the "Server" and "Custom Central Server" lists.
i.e. right-click server and select "Remove from list"

Describe alternatives that have been considered

Until now users have to edit the ini file to clean up their server lists.

@DominikSchaller
Copy link

What is the use case? Why would someone need to delete a server? If it's not used, it would just move to the end of the list.

@pgScorpio
Copy link
Contributor Author

What is the use case? Why would someone need to delete a server? If it's not used, it would just move to the end of the list.

For one "How do I delete a server" is a question I regularly get on several communities. Often because they accidentally entered a wrong server address and regularly, again accidentally, keep clicking this wrong server. And sometimes they just want to delete the server because the server no longer exists (also logical).

Also accidentally clicking the wrong server happens regularly with people that have participated in an event, and therefore sometimes end up in another event where they are not supposed to be (if this was a one time event, they should be able to delete the server after the event, right?)

Especially if most of the server addresses are IP numbers a mistake is easily made.
Unfortunately accidentally clicking the wrong server brings it back to the top of the list, increasing the chance of making the same mistake again.

In all of these cases it would be very appreciated if they just could delete that server address in a simple way.

@gilgongo
Copy link
Member

gilgongo commented Apr 2, 2021

Just to be clear, you are talking about the combo boxes that store a list of previously selected addresses, is that right?

combo

image

If so, then there isn't a standard way of "editing" combo boxes so I guess this would need to be a custom component. BTW best not implemented with a context menu in case that causes trouble for the Android crew, but also because "intermittent" use of right click is not generally recommended,

@gilgongo gilgongo added this to Triage in Tracking (old) via automation Apr 2, 2021
@gilgongo gilgongo changed the title Add option to delete stored (central)servers. Add option to delete stored servers from combo lists Apr 2, 2021
@pgScorpio
Copy link
Contributor Author

Just to be clear, you are talking about the combo boxes that store a list of previously selected addresses, is that right?

combo

image

If so, then there isn't a standard way of "editing" combo boxes so I guess this would need to be a custom component. BTW best not implemented with a context menu in case that causes trouble for the Android crew, but also because "intermittent" use of right click is not generally recommended,

Yes, you are correct. I'm talking about the combo boxes.

I personally would stick to the standard ways of doing things, In this case this is a context menu. right-click on mouse systems (or ctrl-click on some mac's), two finger click on a trackpad, or long press on a touchscreen (Android),

@ann0see ann0see added the feature request Feature request label Apr 2, 2021
@gilgongo
Copy link
Member

gilgongo commented Apr 2, 2021

In this case this is a context menu.

The trouble with having a context menu for this is that nothing else in Jamulus is using one. So it's not "the standard ways of doing things", I'm afraid. Context menus really need to be "all or nothing" if the user isn't going to be playing a game of right click hide and seek in the app ("Does the context menu work here? No. OK, maybe here? No. Here? Yes!").

Even if we had a convention in Jamulus for context menu control, combo boxes don't usually have that, so the user may not think to use it. Better to make the removal option explicit in the UI in some way really. But there is a weak (if not very obvious) convention used in some apps (eg the Mac's network finder I think), where the act of selecting the item in the field, then back-spacing over it signals removal from the list.

Either way, it's potentially quite a big job if it involves a custom component I think, but I don't know for sure.

@softins
Copy link
Member

softins commented Apr 2, 2021

Well for anyone who wants to have a go at implementing it, there are some suggestions at these links:

@dcorson-ticino-com
Copy link
Contributor

dcorson-ticino-com commented Apr 4, 2021

Comboboxes don't usually have this possibility and using a method that is not usually used on any UI item is even farther from being intuitive. I think the possibilities are context menu or not at all, in which case the entry falls off the end of the list when it is long enough.
I will look at using a context menu in the next week.
Note that for some strange reason there is already a right click context menu on the connect combobox with 7 entries, but everything is grayed out but paste. I can't find where it is defined and would replace it with just a 1 liner for delete.

@gilgongo
Copy link
Member

gilgongo commented Dec 8, 2021

This has now been implemented in the settings so we should probably do the same for the connection window:

combodelete.mp4

@gilgongo gilgongo moved this from Triage to Backlog in Tracking (old) Dec 8, 2021
@pljones
Copy link
Collaborator

pljones commented Dec 8, 2021

Note your video depends on use of the mouse. Certainly for Windows and Linux, you need #2129 to get it to work with the keyboard.

@pgScorpio
Copy link
Contributor Author

Also I still wonder why this is implemented in such an unusual way?
Why not use a context menu, as is standard on all platforms?
I already got a lot of questions from a lot of users on how to delete no longer used (directory) servers from the list. They all tried to get a context menu, but none of them figured out this way of deleting themselves.

@gilgongo
Copy link
Member

gilgongo commented Dec 9, 2021

As I explained: #1421 (comment)

A casual look though some other desktop apps I have also shows that none have context menus for this, but most have "select delete" patterns. As noted too though, we would have preferred a simple "x" mini button.

@ann0see
Copy link
Member

ann0see commented Apr 22, 2022

Any updates here? I think the solution is to implement it similar to how the directories can be deleted?

@pgScorpio
Copy link
Contributor Author

@ann0see

Any updates here? I think the solution is to implement it similar to how the directories can be deleted?

Yes, that would be a solution, but still I think that's a strange, unusual way, and as CONTRIBUTING says "Features should be usable in the sense that they act as expected to somebody who does not have a technical background."

And, in my opinion the current way is far from "as expected" since I have to explain this way to everyone "who does not have a technical background" again and again...

@gilgongo
Copy link
Member

Out of interest, why are the people who ask you having to delete the values in the list in the first place?

@ann0see
Copy link
Member

ann0see commented Apr 23, 2022

If someone hosts a server at home and gives out the IP each time, there would be a long list of unusable IPs in the connect window.

@ann0see
Copy link
Member

ann0see commented Apr 23, 2022

So would you prefer a "ₓ" delete button?

@gilgongo
Copy link
Member

gilgongo commented Apr 23, 2022

So that hitting "x" (preferably inside the field) would do the same thing as select/delete, then that would be clear, yes. The problem is that there isn't really a convention to have dropdowns that allow add/remove: https://ux.stackexchange.com/questions/64036/deleting-items-from-a-dropdown-menu and I don't think a separate "management UI" is really warranted.

@dcorson-ticino-com
Copy link
Contributor

Why would we want to add a completely non-standard delete?
I had implemented delete using the right click menu a long time ago, but although a very standard way to do things, it was not OK because it is not used as such elsewhere in Jamulus.
Where is the logic?

@gilgongo
Copy link
Member

gilgongo commented Apr 23, 2022

@dcorson-ticino-com Having a context menu on a drop-down menu in order to delete items in that menu is not a standard way of doing things. A least, I know of no application that does it, or if there are I was not aware of it. An "x" visible in the UI makes you aware of the function and is (albeit uncommonly) sometimes a feature seen in other apps. (see also my comment I think you allude to about Jamulus #1421 (comment))

That said, select/delete is an extremely common pattern in other UI contexts, just not very common in drop-downs.

@ann0see
Copy link
Member

ann0see commented Apr 23, 2022

Just a quick note:

Mozilla does similar to us in the custom directories field
https://support.mozilla.org/en-US/questions/1220341

I've seen x buttons on macOS

Right click doesn't work well on mobile/touchscreens.

@dcorson-ticino-com
Copy link
Contributor

Right click doesn't work well on mobile/touchscreens.

I guess that when they have good Ethernet connectivity that will be an issue, but I don't see it coming.

@pgScorpio
Copy link
Contributor Author

pgScorpio commented Apr 23, 2022

Having a context menu on a drop-down menu in order to delete items in that menu is not a standard way of doing things. A least, I know of no application that does it

AFAIK almost any app uses right-click menus for things like delete, rename, etc.

Right click doesn't work well on mobile/touchscreens.

No but long-press does exactly the same (At least on android and iOS).

@pgScorpio
Copy link
Contributor Author

@gilgongo

Out of interest, why are the people who ask you having to delete the values in the list in the first place?

I already explained this in a earlier post... ;=)

In most cases because they entered a wrong address, or they want to delete "one time event" servers.

@gilgongo
Copy link
Member

gilgongo commented Apr 23, 2022

AFAIK almost any app uses right-click menus for things like delete, rename, etc.

Not for drop-down menus. But even if that WAS the case, the wider point is that having a context menu for this would mean we'd need to have context menus for everything, if the user isn't going to be playing a game of right click hide and seek in the app ("Does the context menu work here? No. OK, maybe here? No. Here? Yes!").

Context menus are useful in some apps like DAWS or design tools that have large numbers of often contextual commands that can be applied to "objects" in the UI. But Jamulus is not one of those apps.

There is nothing wrong with a "clear" control visible if we need that, as @ann0see demonstrates in his examples.

@pljones pljones moved this from Backlog to Triage in Tracking (old) Oct 5, 2022
@pljones pljones added this to the Release 3.11.0 milestone Aug 12, 2023
@pljones
Copy link
Collaborator

pljones commented Aug 12, 2023

If we go for the above idea, this should be pretty easy to do.

@pljones pljones self-assigned this Aug 28, 2023
ann0see added a commit that referenced this issue Feb 12, 2024
Fixes #1421: Add "delete server" button to connect dialog
@pljones pljones mentioned this issue Jul 22, 2024
62 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Feature request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

7 participants