Fix drawer message according to desings #19056
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
clauxx
requested review from
flexsurfer,
Parveshdhull,
J-Son89 and
ibrkhalil
clauxx
commented
Feb 29, 2024
Jenkins BuildsClick to see older builds (53)
|
J-Son89
reviewed
Feb 29, 2024
src/status_im/contexts/chat/messenger/messages/content/view.cljs
Outdated
Show resolved
Hide resolved
clauxx
force-pushed
the
cl-19053-message-bottom-sheet
branch
from
b989230
to
892278d
Compare
J-Son89
approved these changes
Mar 1, 2024
79% of end-end tests have passedFailed tests (9)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (38)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityMultipleDevicePR:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
|
pavloburykh
force-pushed
the
cl-19053-message-bottom-sheet
branch
from
9d46725
to
9ea2dca
Compare
98% of end-end tests have passedExpected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (47)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityMultipleDevicePR:
Class TestDeepLinksOneDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
|
|
@clauxx thanx for the PR. Could you please rebase and resolve existing conflicts so we proceed with testing? Thank you. |
clauxx
force-pushed
the
cl-19053-message-bottom-sheet
branch
2 times, most recently
from
9feb5e6
to
9b181c9
Compare
|
@pavloburykh the conflicts are fixed 👍 |
|
added #19164 as fixed |
83% of end-end tests have passedFailed tests (7)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (40)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePRTwo:
Class TestDeepLinksOneDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePR:
|
@clauxx thank you! Could you please also take a look why integration tests are failing https://ci.status.im/job/status-mobile/job/prs/job/tests/job/PR-19056/10/consoleText ? |
Sometimes the tests job seems to times out. I re-ran it and it passed now. |
86% of end-end tests have passedFailed tests (1)Click to expandClass TestCommunityMultipleDeviceMerged:
Passed tests (6)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
|
|
@VolodLytvynenko if you look at this design for instance, when the message doesn't fit it goes below the bottom sheet. |
clauxx
force-pushed
the
cl-19053-message-bottom-sheet
branch
2 times, most recently
from
cc43fdc
to
b9392e3
Compare
|
Got it @clauxx , thank you. Now I see. The current case occurs only when a user taps their own message and only when using an iOS device. However there is opposite thing , in the case of Android or when an iOS user long-taps a long message from other users, the spacing is shown. Could you fix it please? ISSUE: Spacing between message preview and drawer is shown on Android or when a user on iOS long taps long text messages of other users.Steps:
Actual result:
IOS record ios_drawer.mp4Android record android_drawer.mp4Expected result:No spacing is shown when long text message is long tapped |
|
@VolodLytvynenko well I think the reason for that is that own messages have more options in the menu and less space for the message, so there's no gap if the message doesn't fit. It is basically dependent on the available space for the message. In the android example you showed, the screen is longer so both fit together with the space. |
Hi @clauxx thank you for clarification. No other additional issues from my side. PR can be merged. Thanx! |
clauxx
force-pushed
the
cl-19053-message-bottom-sheet
branch
from
b9392e3
to
73b4294
Compare
|
@Francesca-G can you have a look as well please? |
|
@Francesca-G please have a look again 
|
alwx
approved these changes
Mar 19, 2024
Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>
Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>
Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>
Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>
Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>
Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>
clauxx
force-pushed
the
cl-19053-message-bottom-sheet
branch
from
fe73b8b
to
b71c59e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #19053
fixes #19164
Summary
Fix issues with the message component when shown above the bottom sheet after long-press, based on the design feedback
Testing notes
Platforms
Areas that maybe impacted
Functional
Steps to test
Before and after screenshots comparison