131 members profile picture #615
Conversation
volkramweber
requested review from
NikoHadouken,
steffenkleinle and
Stift
as code owners
There was a problem hiding this comment.
Amazing work, works flawlessly! Well done :)
Minor: I am not sure whether we should display the full error here as it gets quite lengthy and might be confusing. We should at least handle offline errors differently I guess (in line with how its already handled in the error screen by making this reusable
)
| child: Stack( | ||
| alignment: Alignment.center, | ||
| children: [ | ||
| CircleAvatar( |
There was a problem hiding this comment.
🤔 Not sure if I'd expect something to happen if I press the circle avatar/profile icon (probably the same as when pressing Foto bearbeiten). Not sure here though.
There was a problem hiding this comment.
Since this was not implemented in the click dummy, I did not implement it either.
Could it be misleading if the user clicks only to see their profile image?
| ProfileImageUploader( | ||
| profile: widget.profile, | ||
| onProfileUpdated: widget.onProfileUpdated, | ||
| onProcessing: _setProcessing, | ||
| ), | ||
| if (widget.profile.image != null) | ||
| ProfileImageDeleter( | ||
| profile: widget.profile, | ||
| onProfileUpdated: widget.onProfileUpdated, | ||
| onProcessing: _setProcessing, | ||
| ), |
There was a problem hiding this comment.
🤔 Should we perhaps display those two options in a row next to each other instead of below each other to occupy less space?
There was a problem hiding this comment.
Could be a good idea, but implemented it like it was prepared in the figma/click dummy.
| @@ -190,6 +190,17 @@ | |||
| "profiles": "Mitglieder", | |||
| "label": "Mitglieder", | |||
| "noResult": "Leider gab es einen Fehler beim Laden des Profils.", | |||
| "profileImage": { | |||
| "update": "Foto bearbeiten", | |||
There was a problem hiding this comment.
🤔 Not sure if bearbeiten is actually the right word here. If there is none set, I would expect it to be Profilbild hinzufügen and if a picture is set, perhaps Profilbild ändern? But I guess that should be checked first.
| "update": "Foto bearbeiten", | |
| "update": "Profilbild bearbeiten", |
There was a problem hiding this comment.
I get your point. But as far there is no further specification, I would keep it like this.
The text is taken from the figma file, so I would keep it also.
I changed it to: |
| @@ -20,7 +20,7 @@ class NewsDetailScreen extends StatelessWidget { | |||
| final theme = Theme.of(context); | |||
| return FutureLoadingScreen<NewsModel?>( | |||
| load: () => fetchNewsById(newsId), | |||
| buildChild: (NewsModel? news) { | |||
| buildChild: (NewsModel? news, _) { | |||
There was a problem hiding this comment.
🤔 This is kinda annoying, but I also did not find a way to avoid this :/
|
I guess this is also due to the click dummy/figma design but the picture deletion alert has a real iOS look which does not fit at all to the rest of the material design app imo. |
|
@steffenkleinle I did not find any alert like this in the figma, so I just used the default and added styles from our theme. But I am totally not sure if this is good, and if not, how to do it better. @JuliaFreitagBGSt @leagerndt Could you please add a design for this alert? Or how should we proceed? |
I think you can just follow the material design guide on dialogs: https://m2.material.io/components/dialogs |
|
@volkramweber @steffenkleinle thanks for your work this far. Could you please put this issue back into refinement? |
Short Description
Allows the user to upload and delete profile picture.
Proposed Changes
Side Effects
None.
Testing
Resolved Issues
Fixes: #131