Skip to content

Simplifying pinned messages #5807

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
Merged
mahibi merged 11 commits into from
Feb 5, 2026
Merged

Simplifying pinned messages #5807

mahibi merged 11 commits into from
Feb 5, 2026

Conversation

@rapterjet2004
Copy link
Contributor

@rapterjet2004 rapterjet2004 commented Feb 4, 2026 •
edited by AndyScherzinger
Loading

This PR will

  • replace/move the button to open options
  • modify pinned message design
  • fix hardcoded strings
  • add logic to differentiate in headline who pinned it (match web behavior)
  • fix ANR crashes

🖼️ Screenshots

message from other...

pinned by hisself pinned by me
grafik grafik

message from myself

pinned by hisself pinned by myself
grafik grafik

🏁 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?)

@rapterjet2004 rapterjet2004 requested a review from mahibi February 4, 2026 16:46
@rapterjet2004 rapterjet2004 self-assigned this Feb 4, 2026
@rapterjet2004 rapterjet2004 added the 3. to review Waiting for reviews label Feb 4, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2026

Codacy

Lint

TypemasterPR
Warnings9898
Errors00

SpotBugs

CategoryBaseNew
Bad practice66
Correctness1010
Dodgy code5454
Internationalization33
Malicious code vulnerability33
Performance44
Security11
Total8181

@AndyScherzinger
Copy link
Member

AndyScherzinger commented Feb 4, 2026

More compact now and expected to paddings 👍
Any chance to use a regular top-end aligned 3-dot menu button instead of a drawer icon and fab-like bottom attached one?

Could be a follow up for sure, any improvement would be great to get in - for @mahibi to decide if though.

BTW web interface doesn't even show who pinned it, no?

@mahibi
Copy link
Collaborator

mahibi commented Feb 5, 2026

i will take over and modify this to get it in for RC2

@mahibi
Copy link
Collaborator

mahibi commented Feb 5, 2026

@rapterjet2004 i modified the PR, hope thats okay for you..

I noticed the ANR was not yet fixed by just using the plain Composable!
I modified some flow handling (see commit messages) which improved the situation a lot.

However one ANR still happened to me, but it was during heavy testing (unpin ~30 messages in a row as fast as i could). This should happen because the unpin actions triggers getRoom. The scope/dispatcher handling should be improved in the future together with better MVVM architecture, but it's nothing for this PR

@mahibi mahibi assigned mahibi and unassigned rapterjet2004 Feb 5, 2026
@mahibi
Copy link
Collaborator

mahibi commented Feb 5, 2026

@rapterjet2004 i cant set you as reviewer. Feel free to have a look..

@mahibi mahibi added this to the 23.1.0 milestone Feb 5, 2026
@mahibi
Copy link
Collaborator

mahibi commented Feb 5, 2026

/backport to stable-23.0

@mahibi mahibi mentioned this pull request Feb 5, 2026
Closed
5 tasks
@rapterjet2004
Copy link
Contributor Author

rapterjet2004 commented Feb 5, 2026

approve

rapterjet2004 and others added 10 commits February 5, 2026 17:06
@rapterjet2004 @AndyScherzinger
Simplifying pinned messages
fb52412 
Signed-off-by: rapterjet2004 <juliuslinus1@gmail.com>
@mahibi @AndyScherzinger
use 3dot button inside box / add shadow / avoid ripple
90121b6 
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi @AndyScherzinger
add styling to pinned message headline
5bf55e7 
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi @AndyScherzinger
fix hardcoded strings
374f3da 
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi @AndyScherzinger
use Dispatchers.IO for getIndividualMessageFromServer
8c40206 
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi @AndyScherzinger
hopefully fix ANR issues by using flatMapLatest to cancel previous pi…
7c7019a 
…nned message requests

getConversationFlow emits too often, causing multiple overlapping
calls to getIndividualMessageFromServer (i guess this caused the ANR issues).
Using flatMapLatest ensures
that only the latest conversation triggers a network call and cancels previous requests.
This should hopefully avoid the ANR issues.

TODO: improve architecture
-> reduce unnecessary getConversationFlow triggers
-> better MVVM (too many things are inside Activity)

I wont do further changes now to avoid too many changes for v23RC2

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi @AndyScherzinger
differentiate "pinned by" messages
dc9390c 
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@AndyScherzinger
test: Add previews
8c33531 
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
@AndyScherzinger
style(dark): Fix message color
5a47ef3 
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
@mahibi @AndyScherzinger
resolve ktlint warning
2e5478e 
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@AndyScherzinger AndyScherzinger force-pushed the pr-5774-1 branch 2 times, most recently from b4bb0e8 to a2471be Compare February 5, 2026 16:23
@AndyScherzinger
style: Optimize top/end spacing of pinned message and update colors
8ab7068 
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
@mahibi mahibi merged commit 3ebb4dd into master Feb 5, 2026
14 of 17 checks passed
@mahibi mahibi deleted the pr-5774-1 branch February 5, 2026 16:45
@backportbot backportbot bot mentioned this pull request Feb 5, 2026
Merged
@github-actions
Copy link
Contributor

github-actions bot commented Feb 5, 2026

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

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

@mahibi mahibi Awaiting requested review from mahibi

Assignees

@mahibi mahibi

Labels

3. to review Waiting for reviews

Projects

None yet

Milestone

23.1.0

Development

Successfully merging this pull request may close these issues.

3 participants

@rapterjet2004 @AndyScherzinger @mahibi