Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Conversation avatars #2712

Merged
merged 38 commits into from
May 1, 2023
Merged

Conversation avatars #2712

merged 38 commits into from
May 1, 2023

Conversation

mahibi
Copy link
Collaborator

@mahibi mahibi commented Jan 20, 2023 •
edited
Loading

resolve #2555, #1024

🖼️ Screenshot + Video

grafik

conversationAvatarExample.mp4

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not needed
  • 🔖 Capability is checked or not needed
  • 🔙 Backport requests are created or not needed: /backport to stable-xx.x
  • 📅 Milestone is set
  • 🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

@mahibi
Copy link
Collaborator Author

mahibi commented Jan 20, 2023

/rebase

@mahibi mahibi force-pushed the feature/2555/conversation-avatars branch from 1c74506 to 71f3227 Compare January 20, 2023 11:25
@mahibi

This comment was marked as outdated.

Sign in to view
@mahibi mahibi added help wanted Extra attention is needed 1. to develop Accepted and waiting to be taken care of (should be only set by nextcloud employees) don't merge labels Jan 20, 2023
@mahibi mahibi marked this pull request as draft January 20, 2023 11:41
@AndyScherzinger AndyScherzinger force-pushed the feature/2555/conversation-avatars branch from 4cce6ca to e9d434b Compare January 26, 2023 10:54
@AndyScherzinger

This comment was marked as outdated.

Sign in to view
@mahibi

This comment was marked as outdated.

Sign in to view
@AndyScherzinger

This comment was marked as outdated.

Sign in to view
@mahibi mahibi force-pushed the feature/2555/conversation-avatars branch from e9d434b to 7e98bfd Compare February 20, 2023 12:41
@mahibi
Copy link
Collaborator Author

mahibi commented Mar 16, 2023

/rebase

@nextcloud-command nextcloud-command force-pushed the feature/2555/conversation-avatars branch from 7e98bfd to b2b4474 Compare March 16, 2023 19:51
@mahibi mahibi force-pushed the feature/2555/conversation-avatars branch from b2b4474 to 21079af Compare April 17, 2023 14:35
@mahibi mahibi changed the title Move image picker logic to new class 'PickImage' Conversation avatars Apr 19, 2023
@mahibi mahibi force-pushed the feature/2555/conversation-avatars branch 4 times, most recently from e9377b3 to de90120 Compare April 26, 2023 12:00
@mahibi mahibi removed help wanted Extra attention is needed don't merge labels Apr 27, 2023
@mahibi mahibi force-pushed the feature/2555/conversation-avatars branch from de90120 to 0c18761 Compare April 27, 2023 10:24
@AndyScherzinger AndyScherzinger added 2. developing Work in progress and removed 1. to develop Accepted and waiting to be taken care of (should be only set by nextcloud employees) labels Apr 27, 2023
@mahibi mahibi self-assigned this Apr 28, 2023
@mahibi mahibi removed the 2. developing Work in progress label Apr 28, 2023
mahibi added 18 commits May 1, 2023 12:25
@mahibi
pass "replace" as parameter to load*Avatar methods
076b9b2 
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
fix to always load newest avatar image for chat (disable cache)
c880378 
also disable placeholder (otherwise it's flickering on every load)

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
ignore cache when replacing image
2ecea9e 
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
only reload avatars in conversation list on initial loading, else use…
b4256e5 
… from cache

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
remove unused resource
e9a9d85 
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
use icons for conversation appbar edit view
dcb22c8 
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
change CachePolicy for replace to WRITE_ONLY
6b98a19 
this seems to avoid that on a second load without to use replace, an old image from cache is loaded

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
remove placeholder for loadAvatarInternal
effbd28 
this looks smoother when loading the avatars (less flickering)
"error" and "fallback" take over if something goes wrong.

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
avoid UninitializedPropertyAccessException for optionsMenu
e2c7d57 
otherwise:

E  FATAL EXCEPTION: main
    Process: com.nextcloud.talk2, PID: 13169
    io.reactivex.exceptions.UndeliverableException: The exception could not be delivered to the consumer because it has already canceled/disposed the flow or the exception has nowhere to go to begin with. Further reading: https://github.com/ReactiveX/RxJava/wiki/What's-different-in-2.0#error-handling | kotlin.UninitializedPropertyAccessException: lateinit property optionsMenu has not been initialized
    	at io.reactivex.plugins.RxJavaPlugins.onError(RxJavaPlugins.java:367)
    	at io.reactivex.android.schedulers.HandlerScheduler$ScheduledRunnable.run(HandlerScheduler.java:126)
    	at android.os.Handler.handleCallback(Handler.java:942)
    	at android.os.Handler.dispatchMessage(Handler.java:99)
    	at android.os.Looper.loopOnce(Looper.java:226)
    	at android.os.Looper.loop(Looper.java:313)
    	at android.app.ActivityThread.main(ActivityThread.java:8757)
    	at java.lang.reflect.Method.invoke(Native Method)
    	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:571)
    	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1067)
    Caused by: kotlin.UninitializedPropertyAccessException: lateinit property optionsMenu has not been initialized
    	at com.nextcloud.talk.conversation.info.ConversationInfoActivity.showOptionsMenu(ConversationInfoActivity.kt:214)
    	at com.nextcloud.talk.conversation.info.ConversationInfoActivity$fetchRoomInfo$1.onNext(ConversationInfoActivity.kt:670)
    	at com.nextcloud.talk.conversation.info.ConversationInfoActivity$fetchRoomInfo$1.onNext(ConversationInfoActivity.kt:652)
    	at io.reactivex.internal.operators.observable.ObservableObserveOn$ObserveOnObserver.drainNormal(ObservableObserveOn.java:201)
    	at io.reactivex.internal.operators.observable.ObservableObserveOn$ObserveOnObserver.run(ObservableObserveOn.java:255)
    	at io.reactivex.android.schedulers.HandlerScheduler$ScheduledRunnable.run(HandlerScheduler.java:124)
    	at android.os.Handler.handleCallback(Handler.java:942) 
    	at android.os.Handler.dispatchMessage(Handler.java:99) 
    	at android.os.Looper.loopOnce(Looper.java:226) 
    	at android.os.Looper.loop(Looper.java:313) 
    	at android.app.ActivityThread.main(ActivityThread.java:8757) 
    	at java.lang.reflect.Method.invoke(Native Method) 
    	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:571) 
    	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1067) 

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
use dark avatars for dark mode in ChatActivity
c641d51 
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
only reload 1:1 conversation avatars in conversation list on initial …
9ced549 
…loading, else use from cache

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
rename packages, add license comments
2604522 
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
use icons for profile edit appbar
0b5f38f 
to use same design as for conversation info editing

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
use avatar version of conversations to avoid unnecessary reloading/fl…
d560a4a 
…ickering

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
use cache for user avatars again (ignore flickering)
898728c 
in the future, avatarVersion might be available for user avatars as well..

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
fix to show placeholder avatar when none is set by user
050f0f4 
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
WIP. add themed placeholders for conversationAvatars
28046c2 
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi
remove conversation avatar from chatActivity.
1301d62 
only show users avatars.
conversation avatars were removed again for now, because this would need more work to add custom view to the supportActionBar in order to show themed drawables.

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi mahibi force-pushed the feature/2555/conversation-avatars branch from 3ffd801 to 1301d62 Compare May 1, 2023 10:25
@mahibi
Copy link
Collaborator Author

mahibi commented May 1, 2023

Fine except for the backwards compatible behavior on v26 and below, see screenshot posted earlier.

@AndyScherzinger works fine for me on NC 25.
Or do you mean android api version??

Otherwise i changed some things again, so please test/review again if you like to.

@mahibi mahibi marked this pull request as ready for review May 1, 2023 10:59
@mahibi mahibi added 3. to review Waiting for reviews and removed 2. developing Work in progress labels May 1, 2023
@AndyScherzinger
Reformat java code
248aa69 
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
@AndyScherzinger
Copy link
Member

@AndyScherzinger works fine for me on NC 25.

@mahibi I meant Nc26, but works fine now 👍 Code looks good - feel free to merge 🚀

@github-actions
Copy link

github-actions bot commented May 1, 2023

Codacy

Lint

TypemasterPR
Warnings109109
Errors00

SpotBugs

CategoryBaseNew
Correctness99
Dodgy code163163
Internationalization55
Malicious code vulnerability33
Performance1111
Security22
Total193193

@github-actions
Copy link

github-actions bot commented May 1, 2023

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/2712-talk.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud Talk app.

@mahibi mahibi merged commit be216c4 into master May 1, 2023
@delete-merged-branch delete-merged-branch bot deleted the feature/2555/conversation-avatars branch May 1, 2023 15:21
@mahibi mahibi mentioned this pull request May 4, 2023
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndyScherzinger AndyScherzinger AndyScherzinger approved these changes

@tobiasKaminsky tobiasKaminsky Awaiting requested review from tobiasKaminsky

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

Assignees

@mahibi mahibi

Labels
3. to review Waiting for reviews
Projects
None yet
Milestone
17.0.0
Development

Successfully merging this pull request may close these issues.

🖼️ Conversation avatars
3 participants
@mahibi @AndyScherzinger @timkrueger