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

GUI improvement: Client Settings Page - Contextual Sorting when clicking on column headers #4143

Closed
gspannu opened this issue Jan 16, 2022 · 4 comments
Assignees
Milestone

Comments

@gspannu
Copy link

gspannu commented Jan 16, 2022

Client Settings Page.
Clicking on column headers sorts the columns.

Assume the clients are defined by their IP addresses like

192.168.1.10
192.168.1.11
192.168.1.20
192.168.1.31
192.168.1.100
192.168.1.101
192.168.1.205 etc…

Sorting on the Client column should sort it like the above (i.e. mathematically incrementing ).. however, the sorting is actually done as if the data was text

192.168.1.10
192.168.1.100
192.168.1.101
192.168.1.11
192.168.1.20
192.168.1.205
192.168.1.31

Could this small GUI improvement be included in the next build please?

@gspannu gspannu changed the title Client Setting Page - Smarter sorting when clicking on column headers GUI improvement: Client Settings Page - Contextual Sorting when clicking on column headers Jan 16, 2022
@ainar-g ainar-g added this to the v0.107.3 milestone Jan 19, 2022
@ainar-g
Copy link
Contributor

ainar-g commented Jan 19, 2022

Thanks for the report! How would you prefer the clients to be sorted when a CIDR, a MAC, or a ClientID is used? The fact is, the client identifiers are text that is interpreted differently depending on the content.

I think we could use the IP comparison if both identifiers are IP addresses and switch to text comparison if either one of them is text, but I'm not sure.

@IldarKamalov, would that be feasible?

@gspannu
Copy link
Author

gspannu commented Jan 20, 2022

Thanks for the report! How would you prefer the clients to be sorted when a CIDR, a MAC, or a ClientID is used? The fact is, the client identifiers are text that is interpreted differently depending on the content.

I think we could use the IP comparison if both identifiers are IP addresses and switch to text comparison if either one of them is text, but I'm not sure.

@IldarKamalov, would that be feasible?

I think mathematical number sort (if any numbers are involved, else text).
The would be the simple approach; although it may not be that simple to implement !

adguard pushed a commit that referenced this issue Jan 20, 2022
Merge in DNS/adguard-home from 4143-clients-sort to master

Updates #4143.

Squashed commit of the following:

commit a4b547e
Merge: d369c11 d82b290
Author: Ildar Kamalov <ik@adguard.com>
Date:   Thu Jan 20 11:58:42 2022 +0300

    Merge branch 'master' into 4143-clients-sort

commit d369c11
Author: Ildar Kamalov <ik@adguard.com>
Date:   Wed Jan 19 16:53:39 2022 +0300

    client: fix sort ip method

commit d767a11
Author: Ildar Kamalov <ik@adguard.com>
Date:   Wed Jan 19 16:23:23 2022 +0300

    client: sort client ids
@ainar-g
Copy link
Contributor

ainar-g commented Jan 20, 2022

@gspannu, I think you mean what is called natural sorting. We'll consider that in the future, but for now the today's edge release applies IP address comparison logic for the appropriate identifiers. Can you please check if our solution works for you?

@gspannu
Copy link
Author

gspannu commented Jan 20, 2022

@gspannu, I think you mean what is called natural sorting. We'll consider that in the future, but for now the today's edge release applies IP address comparison logic for the appropriate identifiers. Can you please check if our solution works for you?

Just tested... IP address sorting works like a charm.
Thanks.

@ainar-g ainar-g closed this as completed Jan 20, 2022
adguard pushed a commit that referenced this issue Jan 21, 2022
Merge in DNS/adguard-home from 4143-clients-sort to master

Updates #4143.

Squashed commit of the following:

commit a4b547e
Merge: d369c11 d82b290
Author: Ildar Kamalov <ik@adguard.com>
Date:   Thu Jan 20 11:58:42 2022 +0300

    Merge branch 'master' into 4143-clients-sort

commit d369c11
Author: Ildar Kamalov <ik@adguard.com>
Date:   Wed Jan 19 16:53:39 2022 +0300

    client: fix sort ip method

commit d767a11
Author: Ildar Kamalov <ik@adguard.com>
Date:   Wed Jan 19 16:23:23 2022 +0300

    client: sort client ids
heyxkhoa pushed a commit to heyxkhoa/AdGuardHome that referenced this issue Mar 20, 2023
Merge in DNS/adguard-home from 4143-clients-sort to master

Updates AdguardTeam#4143.

Squashed commit of the following:

commit a4b547e
Merge: d369c11 d82b290
Author: Ildar Kamalov <ik@adguard.com>
Date:   Thu Jan 20 11:58:42 2022 +0300

    Merge branch 'master' into 4143-clients-sort

commit d369c11
Author: Ildar Kamalov <ik@adguard.com>
Date:   Wed Jan 19 16:53:39 2022 +0300

    client: fix sort ip method

commit d767a11
Author: Ildar Kamalov <ik@adguard.com>
Date:   Wed Jan 19 16:23:23 2022 +0300

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

No branches or pull requests

3 participants