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

Sort contacts by group and name #3334

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Conversation

MizukiTemma
Copy link
Member

Short description

This PR changes the sort order of contacts so that they are first grouped by POI and then sorted by name alphabetically.

Proposed changes

  • Add "location" to ordering

Side effects

  • should be none 🙈

Resolved issues

Fixes: #3252


Pull Request Review Guidelines

@MizukiTemma MizukiTemma added effort: low Should be doable in <4h deadline Needs to be fixed in the given time labels Jan 14, 2025
Copy link
Contributor

@JoeyStk JoeyStk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much! This looks good already! :)

I only ran into one problem. The ordering by location is already working, but the locations are not sorted alphabetically but instead by poi_id. In my case "Test" was sorted before "Entwurf", as it had the lower id.
But, when I tried to find a fix for it, I ran into a few problems and I couldn't find an easy solution. Does someone have an idea? :)

@JoeyStk JoeyStk self-assigned this Jan 14, 2025
@MizukiTemma
Copy link
Member Author

MizukiTemma commented Jan 14, 2025

I only ran into one problem. The ordering by location is already working, but the locations are not sorted alphabetically but instead by poi_id. In my case "Test" was sorted before "Entwurf", as it had the lower id. But, when I tried to find a fix for it, I ran into a few problems and I couldn't find an easy solution. Does someone have an idea? :)

Ohw 😓 I didn't read the issue conversation carefully enough, thought that it's enough to group by POI and after that sort contacs by name whithin each POI group.

As POI objects do not have any name (names are provided by translations) I don't think sorting POI by name is handy enough. It rather makes sense to add filter by POI.

@JoeyStk
Copy link
Contributor

JoeyStk commented Jan 16, 2025

As POI objects do not have any name (names are provided by translations) I don't think sorting POI by name is handy enough. It rather makes sense to add filter by POI.

Yes, exactly. That's why I also didn't find an easy solution for this. Should we wait for the second reviewer for an idea or should we ask UI/UX / Service if it's okay this way? :)

@MizukiTemma
Copy link
Member Author

MizukiTemma commented Jan 17, 2025

@osmers
Should POIs be sorted by their name at any cost? I know it's better but saldy not technically cheap. (and sorting by name means POIs will be sorted in different order depent on the language)

Copy link
Contributor

@charludo charludo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! 🎉

Re: the sorting iussue: maybe we could use this as an opportunity to add client-side sorting to all lists of this kind? I.e. allow users to cklick on any table header field to sort/reverse-sort by that key? AFAIK that should be relatively straight-forward (and a very common / expected feature anyways)

@JoeyStk
Copy link
Contributor

JoeyStk commented Jan 18, 2025

Re: the sorting iussue: maybe we could use this as an opportunity to add client-side sorting to all lists of this kind? I.e. allow users to cklick on any table header field to sort/reverse-sort by that key? AFAIK that should be relatively straight-forward (and a very common / expected feature anyways)

I think this would be the best solution, but let's see what @osmers thinks :) While we were planning the region condition @PeterNerlich and I considered something similiar, so it might me good to implement it in all desired places at the same time :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deadline Needs to be fixed in the given time effort: low Should be doable in <4h
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sort order for contact list view not specified
3 participants