Skip to content
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

[161108] Optimize message styling when there's multiple mentions on top of each other #16505

Merged
merged 3 commits into from
Jul 16, 2023

Conversation

ibrkhalil
Copy link
Contributor

fixes #16108

Summary

Side to side comparison

image


image

QA notes:
Need testing on multiple devices

status: ready

@status-im-auto
Copy link
Member

status-im-auto commented Jul 6, 2023

Jenkins Builds

Click to see older builds (34)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ cf6fe07 #1 2023-07-06 09:38:44 ~6 min ios 📱ipa 📲
✔️ cf6fe07 #1 2023-07-06 09:41:36 ~9 min android-e2e 🤖apk 📲
✔️ d972256 #2 2023-07-06 09:47:41 ~5 min ios 📱ipa 📲
✔️ d972256 #2 2023-07-06 09:47:48 ~5 min android-e2e 🤖apk 📲
✔️ d972256 #2 2023-07-06 09:48:27 ~6 min android 🤖apk 📲
✔️ d972256 #2 2023-07-06 09:49:27 ~7 min tests 📄log
✔️ ea16983 #3 2023-07-06 11:57:54 ~6 min ios 📱ipa 📲
✔️ ea16983 #3 2023-07-06 12:02:43 ~11 min android 🤖apk 📲
✔️ ea16983 #3 2023-07-06 12:02:48 ~11 min android-e2e 🤖apk 📲
✔️ ea16983 #3 2023-07-06 12:07:51 ~16 min tests 📄log
✔️ 1099c80 #4 2023-07-07 14:33:52 ~6 min android 🤖apk 📲
✔️ 1099c80 #4 2023-07-07 14:33:58 ~6 min ios 📱ipa 📲
✔️ 1099c80 #4 2023-07-07 14:33:58 ~6 min android-e2e 🤖apk 📲
✔️ 1099c80 #4 2023-07-07 14:36:14 ~8 min tests 📄log
✔️ 20cf307 #5 2023-07-08 16:06:16 ~5 min ios 📱ipa 📲
✔️ 20cf307 #5 2023-07-08 16:09:42 ~9 min android-e2e 🤖apk 📲
✔️ 20cf307 #5 2023-07-08 16:09:45 ~9 min android 🤖apk 📲
✔️ 20cf307 #5 2023-07-08 16:10:11 ~9 min tests 📄log
becb487 #6 2023-07-12 14:01:23 ~2 min tests 📄log
✔️ becb487 #6 2023-07-12 14:05:03 ~6 min ios 📱ipa 📲
✔️ becb487 #6 2023-07-12 14:05:07 ~6 min android-e2e 🤖apk 📲
✔️ becb487 #6 2023-07-12 14:05:14 ~6 min android 🤖apk 📲
8b6c32f #7 2023-07-12 15:10:29 ~2 min tests 📄log
✔️ 8b6c32f #7 2023-07-12 15:14:06 ~6 min ios 📱ipa 📲
✔️ 8b6c32f #7 2023-07-12 15:14:19 ~6 min android-e2e 🤖apk 📲
✔️ 8b6c32f #7 2023-07-12 15:14:27 ~6 min android 🤖apk 📲
✔️ 0775f9e #9 2023-07-13 06:45:51 ~5 min android 🤖apk 📲
✔️ 0775f9e #9 2023-07-13 06:48:11 ~8 min android-e2e 🤖apk 📲
✔️ 0775f9e #9 2023-07-13 06:48:40 ~8 min tests 📄log
✔️ 0775f9e #9 2023-07-13 06:49:20 ~9 min ios 📱ipa 📲
5c62b5b #10 2023-07-14 11:42:24 ~2 min tests 📄log
✔️ 5c62b5b #10 2023-07-14 11:45:49 ~5 min android 🤖apk 📲
✔️ 5c62b5b #10 2023-07-14 11:46:25 ~6 min ios 📱ipa 📲
✔️ 5c62b5b #10 2023-07-14 11:48:01 ~7 min android-e2e 🤖apk 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 46ce541 #11 2023-07-14 12:16:48 ~5 min android-e2e 🤖apk 📲
✔️ 46ce541 #11 2023-07-14 12:18:41 ~7 min android 🤖apk 📲
✔️ 46ce541 #11 2023-07-14 12:18:44 ~7 min ios 📱ipa 📲
✔️ 46ce541 #11 2023-07-14 12:20:43 ~9 min tests 📄log
✔️ cd960d9 #13 2023-07-16 11:34:07 ~6 min ios 📱ipa 📲
✔️ cd960d9 #13 2023-07-16 11:36:48 ~9 min android-e2e 🤖apk 📲
✔️ cd960d9 #13 2023-07-16 11:36:53 ~9 min android 🤖apk 📲
✔️ cd960d9 #13 2023-07-16 11:37:09 ~9 min tests 📄log

@@ -21,7 +21,8 @@
{:background-color colors/primary-50-opa-10
:padding-horizontal 3
:border-radius 6
:margin-bottom -3})
:margin-bottom -3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I posted screenshots from emulator, Will test on my Pixel 7A

@ibrkhalil
Copy link
Contributor Author

ibrkhalil commented Jul 6, 2023

I posted screenshots from emulator, Will test on my Pixel 7A

Screenshots from Pixel 7A

Screenshot_20230706-142257.png

Screenshot_20230706-142216.png

Copy link
Contributor

@rahulpsingh rahulpsingh left a comment

Choose a reason for hiding this comment

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

Code looks good to me. Please just test once in device!!

@ibrkhalil
Copy link
Contributor Author

Code looks good to me. Please just test once in device!!

This is in Google pixel 7A

#16505 (comment)

@J-Son89 J-Son89 requested a review from OmarBasem July 6, 2023 13:21
@J-Son89
Copy link
Member

J-Son89 commented Jul 6, 2023

looks good to me but I don't know anything about the chat designs/ implementation :) - @OmarBasem can you take a look?

Copy link
Contributor

@OmarBasem OmarBasem left a comment

Choose a reason for hiding this comment

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

lgtm!

@VolodLytvynenko VolodLytvynenko self-assigned this Jul 7, 2023
@VolodLytvynenko
Copy link
Contributor

Blocked by #16520

@status-im-auto
Copy link
Member

91% of end-end tests have passed

Total executed tests: 34
Failed tests: 3
Passed tests: 31
Not executed tests: 2
IDs of not executed tests: 703391,703297 
IDs of failed tests: 702732,702783,702731 

Not executed tests (2)

Click to expand
  • Rerun not executed tests
  • Failed tests (3)

    Click to expand
  • Rerun failed tests

  • Class TestOneToOneChatMultipleSharedDevicesNewUi:

    1. test_1_1_chat_is_shown_message_sent_delivered_from_offline, id: 702783

    Device 2: Find `Text` by `xpath`: `//*[starts-with(@text,'test message')]/ancestor::android.view.ViewGroup[@content-desc='chat-item']//*[@content-desc='message-status']/android.widget.TextView`
    Device 2: `Text` is `Sent`

    critical/chats/test_1_1_public_chats.py:1309: in test_1_1_chat_is_shown_message_sent_delivered_from_offline
        self.errors.verify_no_errors()
    base_test_case.py:182: in verify_no_errors
        pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
     Message status was not changed to Delivered, it's Sent after back up online!
    



    Device sessions

    2. test_1_1_chat_pin_messages, id: 702731

    Test is not run, e2e blocker  
    

    [[reason: [NOTRUN] Pin feature is in development]]

    Class TestGroupChatMultipleDeviceMergedNewUI:

    1. test_group_chat_pin_messages, id: 702732

    Test is not run, e2e blocker  
    

    [[reason: [NOTRUN] Pin feature is in development]]

    Passed tests (31)

    Click to expand

    Class TestActivityCenterContactRequestMultipleDevicePR:

    1. test_activity_center_contact_request_accept_swipe_mark_all_as_read, id: 702851
    Device sessions

    2. test_activity_center_contact_request_decline, id: 702850
    Device sessions

    Class TestActivityMultipleDevicePR:

    1. test_activity_center_mentions, id: 702957
    Device sessions

    2. test_navigation_jump_to, id: 702936
    Device sessions

    3. test_activity_center_reply_read_unread_delete_filter_swipe, id: 702947
    Device sessions

    4. test_activity_center_admin_notification_accept_swipe, id: 702958
    Device sessions

    Class TestOneToOneChatMultipleSharedDevicesNewUi:

    1. test_1_1_chat_emoji_send_reply_and_open_link, id: 702782
    Device sessions

    2. test_1_1_chat_text_message_delete_push_disappear, id: 702733
    Device sessions

    3. test_1_1_chat_delete_via_long_press_relogin, id: 702784
    Device sessions

    4. test_1_1_chat_push_emoji, id: 702813
    Device sessions

    5. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
    Device sessions

    6. test_1_1_chat_edit_message, id: 702855
    Device sessions

    7. test_1_1_chat_message_reaction, id: 702730
    Device sessions

    Class TestGroupChatMultipleDeviceMergedNewUI:

    1. test_group_chat_reactions, id: 703202
    Device sessions

    2. test_group_chat_join_send_text_messages_push, id: 702807
    Device sessions

    3. test_group_chat_offline_pn, id: 702808
    Device sessions

    Class TestCommunityMultipleDeviceMerged:

    1. test_community_several_images_send_reply, id: 703194
    Device sessions

    2. test_community_one_image_send_reply, id: 702859
    Device sessions

    3. test_community_emoji_send_copy_paste_reply, id: 702840
    Device sessions

    4. test_community_mark_all_messages_as_read, id: 703086
    Device sessions

    5. test_community_contact_block_unblock_offline, id: 702894
    Device sessions

    6. test_community_mentions_push_notification, id: 702786
    Device sessions

    7. test_community_message_delete, id: 702839
    Device sessions

    8. test_community_message_send_check_timestamps_sender_username, id: 702838
    Device sessions

    9. test_community_links_with_previews_github_youtube_twitter_gif_send_enable, id: 702844
    Device sessions

    10. test_community_message_edit, id: 702843
    Device sessions

    11. test_community_leave, id: 702845
    Device sessions

    12. test_community_unread_messages_badge, id: 702841
    Device sessions

    Class TestCommunityOneDeviceMerged:

    1. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133
    Device sessions

    2. test_community_copy_and_paste_message_in_chat_input, id: 702742
    Device sessions

    3. test_community_navigate_to_channel_when_relaunch, id: 702846
    Device sessions

    @VolodLytvynenko
    Copy link
    Contributor

    VolodLytvynenko commented Jul 10, 2023

    Hi @ibrkhalil thank you for PR. The issue still reproduced for some devices

    ISSUE 1: Partial overlapping of mentions in messages with multiple lines if the display name is in uppercase

    Steps to reproduce:

    1. Set an uppercase a display name
    2. Open a chat or conversation.
    3. Type a message with multiple lines (at least 3 lines).
    4. Include multiple mentions of uppercase a display name within the message.

    Actual result:

    • When the message contains multiple lines and includes multiple mentions, the mentions partially overlap each other for
      iPhone 11 Pro max, IOS 16
      image

    Pixel 7a, Android 13
    image

    Expected result:

    Mentions should be displayed properly and clearly without overlapping each other

    Devices:

    • iPhone 11 Pro max, IOS 16
    • Pixel 7a, Android 13

    @ibrkhalil
    Copy link
    Contributor Author

    @ibrkhalil
    Copy link
    Contributor Author

    ibrkhalil commented Jul 16, 2023

    Merging this as it's an improvement according to QA review, But this will be investiaged more later .. Maybe when this GitHub issue on RN is fixed.

    @VolodLytvynenko Had a chat with Chu on it and she approved the minor improvement involoved.

    @ibrkhalil ibrkhalil merged commit b2cbed4 into develop Jul 16, 2023
    2 checks passed
    @ibrkhalil ibrkhalil deleted the 16108 branch July 16, 2023 11:40
    @tumanov-alex tumanov-alex mentioned this pull request Aug 3, 2023
    J-Son89 pushed a commit that referenced this pull request Aug 8, 2023
    Add locked input component, tests, styles
    Add translations
    Add duration icons
    
    Remove extra code
    
    Use theme from context
    
    Add missing code
    
    Update styles
    Update gas icon (previous was not reacting to size change)
    Use text from components instead of rn/text
    
    Fix styling for transaction sheet preview, locked input & account selector components
    Fix purple 50 color since it doesn't match design
    
    Work on PR suggestions
    Fix style to be pixel-perfect
    
    Comment-in tests
    
    Fix style
    
    Add docs for locked-input component
    
    Remove extra code
    
    Fixed design discrepancies
    
    Fix font-weight
    
    Fix purple color in account selector
    
    Remove unused icons
    
    Fix linter
    
    Fix tests
    
    fix for airplane mode
    
    [161108] Optimize message styling when there's multiple mentions on top of each other (#16505)
    
    Fix failing mute till test (#16453)
    
    fix navigation to community from discover communities screen (#16702)
    
    Update version to 0.162.3
    
    [#16703] The display name is not resolved in chats for user sender after relogin (#16704)
    
    Mute community
    
    * mute and unmute community
    
    status-im/status-go@dfdaa72...e6187ae
    
    * mute and unmute community and all community chats
    
    status-im/status-go@dfdaa72...3abc86e
    
    * updated statu-go
    
    status-im/status-go@dfdaa72...919123e
    
    * refactored mute chat drawer
    
    status-im/status-go@d3e650d...3af0b17
    
    * refactored mute chat drawer
    
    status-im/status-go@dfdaa72...3af0b17
    
    * fixing mute channels
    
    * fixed mute community channels
    
    * update community chats mute status
    
    status-im/status-go@dfdaa72...dc50ac2
    
    * added mute and unmute community toast
    
    status-im/status-go@dfdaa72...c06f7a6
    
    * unmute community when atleast one community channel is unmuted
    
    status-im/status-go@dfdaa72...e691c47
    
    * updated status-go
    
    status-im/status-go@b2e56f5...c52718c
    
    * updated status-go version v0.162.5
    
    [Fix] Scroll to bottom on editing message (#16630)
    
    This commit fixes (by skipping) the scroll to the bottom of messages when the user edits a message and sends it.
    
    Signed-off-by: Mohamed Javid <[email protected]>
    
    Refactor `Bottom Sheet` to use Theme Context (#16710)
    
    This commit updates "Bottom Sheet" to use the theme (for theme provider) provided on the bottom sheet args when dispatching. This will ensure the theme is passed down to its child components where it can consume and render based on the theme.
    
    Changes done:
    
    In Bottom Sheet:
     - Fix Bottom Sheet to use the correct background colour (neutral-95) for dark mode (as per Figma)
     - Fix the Icon colour for danger in light mode
     - Updated Quo2 Preview to provide an option for the bottom sheet theme
    
    In Action Drawer:
     - Refactor the Action Drawer component to consume theme context
    
    Signed-off-by: Mohamed Javid <[email protected]>
    
    Revert extra commits
    
    Revert extra commits
    
    Revert extra changes
    J-Son89 pushed a commit that referenced this pull request Aug 8, 2023
    Add locked input component, tests, styles
    Add translations
    Add duration icons
    
    Remove extra code
    
    Use theme from context
    
    Add missing code
    
    Update styles
    Update gas icon (previous was not reacting to size change)
    Use text from components instead of rn/text
    
    Fix styling for transaction sheet preview, locked input & account selector components
    Fix purple 50 color since it doesn't match design
    
    Work on PR suggestions
    Fix style to be pixel-perfect
    
    Comment-in tests
    
    Fix style
    
    Add docs for locked-input component
    
    Remove extra code
    
    Fixed design discrepancies
    
    Fix font-weight
    
    Fix purple color in account selector
    
    Remove unused icons
    
    Fix linter
    
    Fix tests
    
    fix for airplane mode
    
    [161108] Optimize message styling when there's multiple mentions on top of each other (#16505)
    
    Fix failing mute till test (#16453)
    
    fix navigation to community from discover communities screen (#16702)
    
    Update version to 0.162.3
    
    [#16703] The display name is not resolved in chats for user sender after relogin (#16704)
    
    Mute community
    
    * mute and unmute community
    
    status-im/status-go@dfdaa72...e6187ae
    
    * mute and unmute community and all community chats
    
    status-im/status-go@dfdaa72...3abc86e
    
    * updated statu-go
    
    status-im/status-go@dfdaa72...919123e
    
    * refactored mute chat drawer
    
    status-im/status-go@d3e650d...3af0b17
    
    * refactored mute chat drawer
    
    status-im/status-go@dfdaa72...3af0b17
    
    * fixing mute channels
    
    * fixed mute community channels
    
    * update community chats mute status
    
    status-im/status-go@dfdaa72...dc50ac2
    
    * added mute and unmute community toast
    
    status-im/status-go@dfdaa72...c06f7a6
    
    * unmute community when atleast one community channel is unmuted
    
    status-im/status-go@dfdaa72...e691c47
    
    * updated status-go
    
    status-im/status-go@b2e56f5...c52718c
    
    * updated status-go version v0.162.5
    
    [Fix] Scroll to bottom on editing message (#16630)
    
    This commit fixes (by skipping) the scroll to the bottom of messages when the user edits a message and sends it.
    
    Signed-off-by: Mohamed Javid <[email protected]>
    
    Refactor `Bottom Sheet` to use Theme Context (#16710)
    
    This commit updates "Bottom Sheet" to use the theme (for theme provider) provided on the bottom sheet args when dispatching. This will ensure the theme is passed down to its child components where it can consume and render based on the theme.
    
    Changes done:
    
    In Bottom Sheet:
     - Fix Bottom Sheet to use the correct background colour (neutral-95) for dark mode (as per Figma)
     - Fix the Icon colour for danger in light mode
     - Updated Quo2 Preview to provide an option for the bottom sheet theme
    
    In Action Drawer:
     - Refactor the Action Drawer component to consume theme context
    
    Signed-off-by: Mohamed Javid <[email protected]>
    
    Revert extra commits
    
    Revert extra commits
    
    Revert extra changes
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    No open projects
    Archived in project
    Development

    Successfully merging this pull request may close these issues.

    Messaging - In Chat - Multiple Mentions in following line causes UI issue
    6 participants