Skip to content

avoid shimmer animations overlay with chat messages #4629

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

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

mahibi
Copy link
Collaborator

@mahibi mahibi commented Jan 17, 2025

fix #4626

The idea to show the progress bar (aka shimmer animation) was that it show not be shown at all if loading messages happens in less than a second (this was a better UX in my opinion).

The idea was to hide the progress bar when
ChatMessageStartState was triggered.

However there can be moment when adapter is still empty after ChatMessageStartState and if in this moment the DELAY_TO_SHOW_PROGRESS_BAR is reached before the adapter is actually filled, the overlay happens. It could be a solution to move the hiding of the progress bar, however then special cases might have to be handled.

For simplicity, the logic for DELAY_TO_SHOW_PROGRESS_BAR is removed. Progress bar is always shown without a delay so it wont be triggered on a later moment and can't overlay the chat.

🏁 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 mahibi added the 3. to review Waiting for reviews label Jan 17, 2025
@mahibi mahibi added this to the 21.0.0 milestone Jan 17, 2025
@mahibi mahibi requested a review from rapterjet2004 January 17, 2025 12:37
@mahibi mahibi self-assigned this Jan 17, 2025
The idea to show the progress bar (aka shimmer animation) was that it show not be shown at all if loading messages happens in less than a second (this was a better UX in my opinion).

The idea was to hide the progress bar when
ChatMessageStartState was triggered.

However there can be moment when adapter is still empty after ChatMessageStartState and if in this moment the DELAY_TO_SHOW_PROGRESS_BAR is reached before the adapter is actually filled, the overlay happens.
It could be a solution to move the hiding of the progress bar, however then special cases might have to be handled.

For simplicity, the logic for DELAY_TO_SHOW_PROGRESS_BAR is removed. Progress bar is always shown without a delay so it wont be triggered on a later moment and can't overlay the chat.

Signed-off-by: Marcel Hibbe <[email protected]>
@mahibi mahibi force-pushed the bugfix/4626/avoidChatLoadingShimmerOverlay branch from 5e69eb1 to 20612c8 Compare January 17, 2025 12:41
Copy link
Contributor

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/4629-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.

Copy link
Contributor

Codacy

Lint

TypemasterPR
Warnings104104
Errors3636

SpotBugs

CategoryBaseNew
Bad practice66
Correctness222222
Dodgy code7171
Internationalization33
Malicious code vulnerability33
Performance44
Security11
Total310310

@mahibi mahibi enabled auto-merge January 17, 2025 15:23
Copy link
Contributor

@rapterjet2004 rapterjet2004 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mahibi Noticed a bug with offline support - approving anyway for your convenience

Edit: Created a issue. Same thing #4643

  1. turned on airplane mode
  2. entered into a conversation without any offline messages saved
  3. Shimmer present when it shouldn't be

Screenshot_20250122-102517

Also when you reenter the conversation the background icon is still there. ( TBH I like the effect, reminds me of backgrounds on Telegram and Whatsapp. Chat backgrounds a future feature?), but I don't know if it's related to your PR or mine when I implemented the feature some time ago.

Screenshot_20250122-102657

@mahibi mahibi merged commit d2d040d into master Jan 22, 2025
18 checks passed
@mahibi mahibi deleted the bugfix/4626/avoidChatLoadingShimmerOverlay branch January 22, 2025 16:37
@rapterjet2004 rapterjet2004 restored the bugfix/4626/avoidChatLoadingShimmerOverlay branch January 22, 2025 16:37
@rapterjet2004
Copy link
Contributor

Oops, didn't realize auto-merge was on. My bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Loading chat" shimmer effect not hidden after chat is loaded
2 participants