-
Notifications
You must be signed in to change notification settings - Fork 79
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
Fixes to pinned messages #16894
Fixes to pinned messages #16894
Conversation
Jenkins BuildsClick to see older builds (14)
|
a571ed3
to
0402861
Compare
@iurimatias @osmaczko @saledjenic friendly reminder to review |
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.
From my side all good, but guess people from the backend team can say more about possible implications, if any.
@@ -3,7 +3,7 @@ import NimQml | |||
import ../item as chat_item | |||
import ../../../../../app_service/service/message/dto/pinned_message | |||
import ../../../../../app_service/service/chat/dto/chat | |||
|
|||
import ../../../../../app_service/service/message/dto/message |
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.
@jrainville you could go without ../../../../../
, actually you can remove those prefixes from every line here, but even if not, we can live with that. ;)
0402861
to
86a6d48
Compare
@saledjenic you were randomly chosen to be the tester for this fix. Thanks
|
What does the PR do
Fixes #16639
Affected areas
Pins
Screenshot of functionality (including design for comparison)
pin-fixes.webm
Impact on end user
Fixes some scenarios of using pinned messages
How to test
Risk
Tick one:
Worst case is issue still happens