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

FEAT(client, ui): Revamped server information dialog #4891

Merged
merged 2 commits into from Mar 28, 2021

Conversation

Krzmbrzl
Copy link
Member

@Krzmbrzl Krzmbrzl commented Mar 28, 2021

The previous dialog was simply a message box with a bunch of HTML in
order to give a little structure to the contents. This approach is very
limiting in terms of UI flexibility though and therefore this commit
replaces the old HTML approach with a dedicated dialog class that uses
proper UI elements instead.

While doing so, the ordering and grouping of information was also
changed in order to make it more suitable for the every-day-user.

Fixes #4733

Checks


Screenshots

Mumble_ServerInfo01
Mumble_ServerInfo02
Mumble_ServerInfo03

For reference this is the old window:
Old

@Krzmbrzl
Copy link
Member Author

@SeerLite please let me know if you think this is sufficient or whether there is something that you think should be changed / improved?

@davidebeatrici
Copy link
Member

I like it, however the UDP (Voice) tab should be hidden and the TCP (Control) one be called TCP (Control + Voice) when UDP is not active.

@Krzmbrzl
Copy link
Member Author

I thought about something like this as well, but concluded that this will probably confuse users more (if they read somewhere to check their UDP statistics, then I think it is better to have that tab available that then just says that voice is currently sent through TCP as well) ☝️

@Krzmbrzl Krzmbrzl force-pushed the feat-revamped-server-info-ui branch from e5b2625 to ab20ba3 Compare March 28, 2021 17:46
@davidebeatrici
Copy link
Member

I agree.

The previous dialog was simply a message box with a bunch of HTML in
order to give a little structure to the contents. This approach is very
limiting in terms of UI flexibility though and therefore this commit
replaces the old HTML approach with a dedicated dialog class that uses
proper UI elements instead.

While doing so, the ordering and grouping of information was also
changed in order to make it more suitable for the every-day-user.

Fixes mumble-voip#4733
@Krzmbrzl Krzmbrzl force-pushed the feat-revamped-server-info-ui branch from ab20ba3 to 2261ea1 Compare March 28, 2021 18:23
@Krzmbrzl Krzmbrzl force-pushed the feat-revamped-server-info-ui branch from 2261ea1 to c0a71a7 Compare March 28, 2021 18:30
@SeerLite
Copy link

Woah that's amazing. It looks a lot better this way! I really should have let you do it from the start (as this was way too advanced for me). Sorry for forgetting about it :s

Thank you so much, all these changes are great. You managed to solve all the issues I had with the original window. This really makes Mumble look a lot more modern than before.

Only improvement I could think of was a small button next to the host and port fields to copy to clipboard, to make it easier to share with friends and contacts. But then I found out about #4847, which would make it as easy in a neater way. I'm not sure if there's any other use case for easy copying of host/port, but there's the idea.

Anyway I think it's perfect like this. Thank you again for everything

@Krzmbrzl
Copy link
Member Author

I really should have let you do it from the start (as this was way too advanced for me). Sorry for forgetting about it :s

No worries. In fact I think I would not have found the motivation to work on this if you hadn't created your PR ^^

Only improvement I could think of was a small button next to the host and port fields to copy to clipboard, to make it easier to share with friends and contacts

You are actually able to select the respective texts and use Ctrl+C on it. Thus you can copy it to clipboard without a button. The only thing I could imagine is a button that lets you copy a complete "Mumble URL" (can be used when starting Mumble to directly connect to that server) as suggested by @Kissaki, but in the end I don't think it is really worth it 🤷

Anyway I think it's perfect like this. Thank you again for everything

Great to hear that 👍
You're welcome :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants