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

IPv6 address should always be displayed first in tooltips #5696

Closed
StrangePeanut opened this issue May 21, 2022 · 8 comments · Fixed by #5714
Closed

IPv6 address should always be displayed first in tooltips #5696

StrangePeanut opened this issue May 21, 2022 · 8 comments · Fixed by #5714
Labels
client feature-request This issue or PR deals with a new feature good first issue Good for first-time contributors ui

Comments

@StrangePeanut
Copy link

StrangePeanut commented May 21, 2022

Description

Not much of an issue or a feature request but an issue nonetheless:

Sometimes (50/50), the dual-stack server's IPv4 address is displayed before the IPv6 address in tooltips, whereas clients always connect via IPv6 as they should. For the sake of consistency and to ease some peoples’ OCD, the IPv6 address should always be displayed before the server’s IPv4 address.

Backstory: I mistakenly assumed our server was preferring IPv4 for some reason. Upon further inspection, my client would connect via IPv6 every time regardless of the tooltip.

Mumble component

Client

OS-specific?

No

@StrangePeanut StrangePeanut added feature-request This issue or PR deals with a new feature triage This issue is waiting to be triaged by one of the project members labels May 21, 2022
@Krzmbrzl Krzmbrzl added client good first issue Good for first-time contributors ui and removed triage This issue is waiting to be triaged by one of the project members labels May 22, 2022
@vimpostor
Copy link
Contributor

I agree that there should be a consistent ordering, but I think IPv4 should always be first, as it is the more conservative choice and mainly also because IPv4 addresses are much more readable than IPv6 ones.

@GuybrushThreepwoodII
Copy link
Contributor

Hello,

a friend and I would like to work on this issue.

@Krzmbrzl
Copy link
Member

@GuybrushThreepwoodII great to hear that! You are of course more than welcome to do so. If you need any help, let me know.

With regards to the ordering of the addresses: Personally, I don't really mind which one is shown first (though I tend to agree with vimpostor's line of argument) so this decision would be up to you as the ones implementing this.

@inodanus
Copy link
Contributor

I'm the friend of @GuybrushThreepwoodII. We have an idea. Instead of changing the ordering we could display both ip addresses on different labels. Splitting Addresses to "IPv4 address" and "IPv6 address". What is your opinion about it?

@StrangePeanut
Copy link
Author

StrangePeanut commented May 23, 2022

Instead of changing the ordering we could display both ip addresses on different labels.

This would be even better in my opinion. It tackles another consistency issue where IPs are displayed either next to each other or stacked depending on the total length.

@Krzmbrzl
Copy link
Member

Yeah separating in different fields and stacking them sounds good to me 👍

@vimpostor
Copy link
Contributor

In what order do you stack them though? 🙊 🤡
(Again I personally think IPv4 should be before the IPv6)

@inodanus
Copy link
Contributor

Thank you for your answers. We will stack IPv4 over IPv6 because it is more relevant for most users in our opinion.

inodanus added a commit to inodanus/mumble that referenced this issue Jun 2, 2022
The "Addresses" field in the ServerItem tooltip shows both ipv4 and ipv6
addresses. The order of addresses in this field is not determined.
In the wild it is randomized. As discussed in mumble-voip#5696 it is better to implement
a concrete ordering to prevent user confusion. To further improve the
readability of the addresses we split the addresses field in two fields
called "IPv4 address" and "IPv6 address". The ipv4 address is now always
shown above the ipv6 address.

Implements mumble-voip#5696

Co-Authored-By: Thomas Pawelek <thomas.pawelek@stud.uni-hannover.de>
inodanus added a commit to inodanus/mumble that referenced this issue Jun 5, 2022
The "Addresses" field in the ServerItem tooltip shows both ipv4 and ipv6
addresses. The order of addresses in this field is not determined.
In the wild it is randomized. As discussed in mumble-voip#5696 it is better to implement
a concrete ordering to prevent user confusion. To further improve the
readability of the addresses we split the addresses field in two fields
called "IPv4 address" and "IPv6 address". The ipv4 address is now always
shown above the ipv6 address.

Implements mumble-voip#5696

Co-Authored-By: Thomas Pawelek <thomas.pawelek@stud.uni-hannover.de>
inodanus added a commit to inodanus/mumble that referenced this issue Jun 5, 2022
The "Addresses" field in the ServerItem tooltip shows both ipv4 and ipv6
addresses. The order of addresses in this field is not determined.
In the wild it is randomized. As discussed in mumble-voip#5696 it is better to implement
a concrete ordering to prevent user confusion. To further improve the
readability of the addresses we split the addresses field in two fields
called "IPv4 address" and "IPv6 address". The ipv4 address is now always
shown above the ipv6 address.

Implements mumble-voip#5696

Co-Authored-By: Thomas Pawelek <thomas.pawelek@stud.uni-hannover.de>
@Krzmbrzl Krzmbrzl linked a pull request Jun 5, 2022 that will close this issue
1 task
Krzmbrzl added a commit that referenced this issue Jul 23, 2022
…m tooltip

The "Addresses" field in the ServerItem tooltip shows both ipv4 and ipv6
addresses. The order of addresses in this field is not determined.
In the wild it is randomized. As discussed in #5696 it is better to implement
a concrete ordering to prevent user confusion. To further improve the
readability of the addresses we split the addresses field in two fields
called "IPv4 address" and "IPv6 address". The ipv4 address is now always
shown above the ipv6 address.

Implements #5696
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client feature-request This issue or PR deals with a new feature good first issue Good for first-time contributors ui
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants