-
Notifications
You must be signed in to change notification settings - Fork 987
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
[#16086] Messaging - Composer - Show current message for user context… #16128
Conversation
Jenkins BuildsClick to see older builds (8)
|
@@ -152,6 +150,14 @@ | |||
(when @show-delivery-state? | |||
[status/status outgoing-status])])]]])))) | |||
|
|||
(defn on-long-press | |||
[message-data context keyboard-shown] | |||
(rf/dispatch [:dismiss-keyboard]) |
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.
Not for this PR, but what's the use of this event in the codebase? Isn't rn/dismiss-keyboard!
sufficient?
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.
effects shouldn't be used directly from view, event should be used instead
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 believe this is not an effect: https://reactnative.dev/docs/keyboard#dismiss
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.
it is from re-frame pov, view must be pure
I think we need to animate the message component itself from its current position? Like when you long-press a message on WhatsApp or Telegram |
probably, we need requirements from design team |
91% of end-end tests have passed
Failed tests (3)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Passed tests (30)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
|
@flexsurfer thanks for the fixes! Question: video_2023-06-06_17-17-54.mp4Also, the first issue with link previews is still reproducible. Can replicate it on Android as well with certain links (e.g. https://www.youtube.com/watch?v=lSTYrMSkF7g&ab_channel=ThePsychedelicMuse) |
it will be difficult to do, but we don't actually need full content, we just need to be sure for which message actions sheet is opened
should be fixed now, thanks |
Thanks for your work @flexsurfer! |
fixes #16086