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 groups alphabetically #1451

Closed
wants to merge 7 commits into from
Closed

Sort groups alphabetically #1451

wants to merge 7 commits into from

Conversation

ImUrX
Copy link
Member

@ImUrX ImUrX commented Jan 31, 2020

Refers to #522
I did the solution just giving sort no arguments and overriding toString to each group's name.
Would like to test, but I dont have any idea on how to do so.

@ImUrX ImUrX requested a review from skjnldsv January 31, 2020 09:02
@codecov
Copy link

codecov bot commented Jan 31, 2020

Codecov Report

Merging #1451 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1451   +/-   ##
=======================================
  Coverage   62.31%   62.31%           
=======================================
  Files           4        4           
  Lines          69       69           
=======================================
  Hits           43       43           
  Misses         26       26

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 377b07b...80ca63e. Read the comment docs.

Signed-off-by: ImUrX <urielfontan2002@gmail.com>
@skjnldsv
Copy link
Member

skjnldsv commented Feb 1, 2020

Hi, and thanks for your pr! :)
So, instead of changing the behaviour entirely, I would suggest adding a setting under the contacts sorting dropdown (in the bottom left settings area)
What do you think?

@skjnldsv skjnldsv added 2. developing Work in progress enhancement New feature or request labels Feb 1, 2020
@skjnldsv skjnldsv added this to the next minor milestone Feb 1, 2020
@ImUrX
Copy link
Member Author

ImUrX commented Feb 1, 2020 via email

ImUrX added 4 commits February 1, 2020 11:23
…calstorage key, add sortGroups in store, use sortGroups each time group menu is generated (maybe refactor that a little?)
Signed-off-by: ImUrX <urielfontan2002@gmail.com>
@ImUrX
Copy link
Member Author

ImUrX commented Feb 1, 2020

i accidentaly deleted package-lock.json :/

@ImUrX
Copy link
Member Author

ImUrX commented Feb 1, 2020

Gonna remake the whole branch so it’s a little more clean

@ImUrX ImUrX closed this Feb 1, 2020
@mburnicki
Copy link

mburnicki commented Feb 2, 2020

I think the entries for "All contacts" and "Contacts without group" should be at the top of the list, just like it's now, and below should be a list of all groups, sorted by name.

I don't know what determines the sequence of groups if they are not sorted by name (as it's now, unfortunately). This is just confusing if you have many groups, and below them groups like "colleagues-local" "colleagues-external", colleagues-formerly", etc.

And, by the way, when editing a contact, also the selection box should where you can assign the groups should provide the list of group names sorted by name, IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants