-
Notifications
You must be signed in to change notification settings - Fork 739
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
Room Settings: Name, Topic, Photo, Aliases, History Visibility #1464
Conversation
onurays
commented
Jun 9, 2020
•
edited
Loading
edited
copy( | ||
roomSummary = async, | ||
newName = roomSummary?.displayName, | ||
newTopic = roomSummary?.topic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I won't mix current data and new data in the view state. The RoomSummary can change during the edition, which could lead to bug
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some remarks
val workerParams = UploadAvatarWorker.Params(sessionId, avatarUri, fileName) | ||
val workerData = WorkerParamsFactory.toData(workerParams) | ||
|
||
val uploadAvatarWork = workManagerProvider.matrixOneTimeWorkRequestBuilder<UploadAvatarWorker>() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same remark than the other PR with uploading avatar, don't think we should use a worker overthere
private fun onRoomAvatarSelected(image: MultiPickerImageType) { | ||
val destinationFile = File(cacheDir, "${image.displayName}_edited_image_${System.currentTimeMillis()}") | ||
val uri = image.contentUri | ||
UCrop.of(uri, destinationFile.toUri()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ucrop again, create helper please
* This class is the view presenting choices for picking avatar. | ||
* It will return result through [Callback]. | ||
*/ | ||
class AvatarSelectorView(context: Context, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why creating again this class, please refact the existing one to be more dynamic, or use a simple dialog?
@@ -63,7 +71,7 @@ abstract class ProfileActionItem : VectorEpoxyModel<ProfileActionItem.Holder>() | |||
override fun bind(holder: Holder) { | |||
super.bind(holder) | |||
holder.view.setOnClickListener(listener) | |||
if (listener == null) { | |||
if (listener == null || !editable) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why adding this? editable bool is for showing/hiding icon. Maybe it's not the good vocabulary, I think it's the one used by Nad, but not sure.
Please fix this reported error:
|
4dedb86
to
e0ea0c1
Compare
}, | ||
{ | ||
_viewEvents.post(RoomProfileViewEvents.Failure(it)) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may put the last parentheses to the new line? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done