Streamline Settings

[szaimen]

This PR is best reviewed like this: https://github.com/nextcloud/contacts/pull/2372/files?diff=unified&w=1

Before	After

Signed-off-by: szaimen szaimen@e.mail.de

[codecov]

Codecov Report

Merging #2372 (4179f25) into master (7ec9046) will not change coverage.
The diff coverage is n/a.

❗ Current head 4179f25 differs from pull request most recent head 435ba0a. Consider uploading reports for the commit 435ba0a to get more accurate results

@@            Coverage Diff            @@
##             master    #2372   +/-   ##
=========================================
  Coverage     69.25%   69.25%           
  Complexity      246      246           
=========================================
  Files            22       22           
  Lines           696      696           
=========================================
  Hits            482      482           
  Misses          214      214           

---

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 7ec9046...435ba0a. Read the comment docs.

[skjnldsv]

Requires @jancborchardt approval.
But there was different plans for the settings: #1703 (comment)

[szaimen]

But there was different plans for the settings: #1703 (comment)

Yes but until it isn't inside a modal which I am not able to do myself, I thought this was a good small fix which improves the usability of the settings by a lot, imho.

[jancborchardt]

Hmm, so since these icons would not be there when moving the settings to the modal, I would say we don’t need to add them here. (And also e.g. the icon is better inside the "Import contacts" button than outside it. :)

[szaimen]

Hmm, so since these icons would not be there when moving the settings to the modal, I would say we don’t need to add them here.

The question is how long it will take to bring the settings into the Modal since the current settings are really bad from a usability perspective, imo. So I'd rather merge this and revert it when bringing it into the settings modall than living with it for another release 😅

[szaimen]

(And also e.g. the icon is better inside the "Import contacts" button than outside it. :)

I've reverted that change, see updated screenshot.

[nimishavijay]

If we are moving to a modal soon then these changes will not be in effect for too long, but I do think the icons look good. Couple of suggestions:

• move the content of the settings menu slightly to the left so that the checkbox and icons in the content are left aligned with the settings icon in the header, and all the text of the header and content is left aligned as well
• "Enter new address book name" can be changed to something shorter like "Add new address book"?

[szaimen]

@nimisha-vijay thanks for the feedback! :)
Addressed both points, see updated screenshots

[artonge]

OK, code wise

[jancborchardt]

Ok – then I just have 2 more pieces of feedback :)

• The sorting setting would be better as the first item in the settings, as it’s the most basic one.
• The address books could do with a header "Address books" to actually understand what they are. (This heading should also have some whitespace above so that section is a bit separated from the two simpler settings above.)

[szaimen]

Done. Please have another look at the updated screenshots @jancborchardt and @nimisha-vijay :)

[szaimen]

@artonge do you mind reviewing the code another time?
I made some small changes...
Thanks! :)