-
Notifications
You must be signed in to change notification settings - Fork 704
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 "Channel list" to network context menu #1802
Conversation
💯 |
client/js/lounge.js
Outdated
text: "Channel list", | ||
data: target.data("target"), | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you move this under the divider?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
client/js/lounge.js
Outdated
if (target.hasClass("lobby")) { | ||
output += templates.contextmenu_item({ | ||
class: "list", | ||
text: "Channel list", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm debating if it would not be more user-friendly to have something like "List channels on this server" or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that’s too long... maybe “List existing channels”?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, it's too long.
Maybe "List Channels" or "List all channels" or "List of Channels" or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really see whats wrong with Channel List, its pretty straight forward. List Channels would fit better to disconnect tho since both are an action.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the issue with "Channel List" or "List Channels" is ... what channels? But none of our solutions fix that problem, really. I'd be fine with any of these other than "List channels on this server"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor label change, but aside that, it works and it's a great thing to have. Thanks!!
(Not for this PR) It would also be nice to add an entry in channel dropdowns to add "List all banned users" that would call /banlist
:)
client/js/lounge.js
Outdated
if (target.hasClass("lobby")) { | ||
output += templates.contextmenu_item({ | ||
class: "list", | ||
text: "Channel list", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"List all channels"
Adding to v2.7.0 milestone as it's super trivial. |
Should be good to go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
No clue if we want this or not, I'm a fan. 🤷♂️
I just think some commands should be more accessible from the UI