#591 Groups in user info#16767
Conversation
Signed-off-by: Daniele Verducci <daniele.verducci@ichibi.eu>
Signed-off-by: Daniele Verducci <daniele.verducci@ichibi.eu>
Signed-off-by: Daniele Verducci <daniele.verducci@ichibi.eu>
|
Hi @penguin86 I'd be fine with the approach for the sake of simplicity for this PR. However in the long(er) run the recycler view would need to be brought back or be a LazyList when migrating to composables I's day or when adding further fields. Because in the long run we would want to show further/all profile fields, like the Talk app does now, see nextcloud/talk-android#5936 - so the simple approach could else have been to go with the non-separated variant and just add another items for the groups or if one would be two implement the two sections to go with a sectioned Recycler view adapter, see for example https://github.com/nextcloud/android/blob/master/app/src/main/java/com/owncloud/android/ui/fragment/contactsbackup/BackupListAdapter.kt#L82-L114 implementing this for the calendar and contacts backup list. Not saying you'd have to do any of the above mentioned thoughts - just to discuss and elaborate on the approach - also given you can't anticipate we would want to add further fields in the future to mirror what the Talk client can and does display. |
|
Hello @AndyScherzinger , |
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
🖼️ Screenshots
Implementation of #591
Notes: the previous implementation used RecyclerView for the list items. That's a sleek implementation and it's very efficient for long lists due to the efficiency in instantiating only the views needed to cover the screen and recycling them when the user scrolls. But in this case, the list barely consists of 6 elements (as I see, all the groups are concatenated in a single item), and in the vast majority of devices they will be all instantiated and drawn anyway, so it doesn't make a real difference.
Obtaining the required result (see "variant 1" in #591 (comment)) with the two "groups" of items while keeping the current structure is possible (playing with cell backgrounds and different row views for the headers), but in my opinion unnecessarily complex, and less maintainable.
I decided to remove the RecyclerView and opt for a simpler implementation, that should be more readable.
Let me know if there is something that could have been done differently and I'll fix it.
🏁 Checklist