Feature: Allow users to delete and like event pictures #315
Conversation
change likes attribute to be a ReferenceList<User> instead of an Int
in PictureOverlay
Update firestore rules to take eventPictures into account.
members and roles were lists instead of maps
Thanks alouche for the help 🚑
Thanks alouche for the help 🚀
|
| } | ||
| }, | ||
| lazy = true) | ||
| if (updateAssociation) { |
There was a problem hiding this comment.
Very good idea to put the boolean, it allows you not to update all the association as you don't need to here
| uid, | ||
| { | ||
| event.eventPictures.remove(uid) | ||
| updateEventWithoutImage( |
There was a problem hiding this comment.
Instead of doing this - that will in firebase for every field to change, you could have just override the image field to "" to avoid unnecessary server computing time
| eventPictures.forEach { pic -> pic.author.fetch(lazy = true) } | ||
| pagerState.scrollToPage(index) |
There was a problem hiding this comment.
This seems a bit too much a fetching right here, but as this is not a big amount of data this is really fine
|
|
||
| if (event == null) { | ||
| Log.e("PictureOverlay", "Event is null") | ||
| Toast.makeText(LocalContext.current, "An error occurred.", Toast.LENGTH_SHORT).show() |
There was a problem hiding this comment.
Consider putting the good error in the Toast if you have it, here : "Event is null"
| Icon( | ||
| Icons.AutoMirrored.Filled.ArrowBack, | ||
| context.getString(R.string.event_details_content_description_arrow_left), | ||
| modifier = Modifier.size(iconSize)) | ||
| modifier = Modifier.size(arrowSize)) | ||
| } | ||
|
|
||
| IconButton( |
There was a problem hiding this comment.
you could remove the arrow right if we are on the lastPage and remove the arrow left if we are on the first page, this should be done very easily with the pageState like if pagerState.currentPageOffsetFraction == -0.5 for example
| Modifier.testTag(EventDetailsTestTags.EVENT_PICTURES_LIKE_BUTTON), | ||
| colors = | ||
| IconButtonDefaults.iconButtonColors( | ||
| contentColor = MaterialTheme.colorScheme.onPrimary)) { |
There was a problem hiding this comment.
Consider hardcoding the colors because as you said, it seems better when being in light mode
There was a problem hiding this comment.
So much valuable insights !
There was a problem hiding this comment.
As I looked a bit the changes you made all the day and your explanations, I don't think there's anything to change here
There was a problem hiding this comment.
Thank you for your review !
From another PR of mine, you can now click on user pictures from the EventDetails. But this menu still lacks responsivity and feels very static. That's why I am adding the possibility to like and delete a picture :)
When you click on a picture, you can see:
-> Icon to like/dislike a picture
-> Counter of likes
-> the author's icon and name
-> a delete button