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

Composer collapsing when editing canceled/done #17785

Merged
merged 26 commits into from
Nov 17, 2023

Conversation

clauxx
Copy link
Member

@clauxx clauxx commented Nov 1, 2023

fixes #17433
fixes #17825

Summary

Fixed issues:

  1. The composer not collapsing when the user cancels editing a message (by pressing x)
  2. The keyboard not hiding when the user cancel editing
  3. The composer not expanding when the user starts editing a message

Platforms

  • Android
  • iOS

Areas that maybe impacted

Functional
  • 1-1 chats
  • public chats
  • group chats

Steps to test

  • Open Status
  • Open chat
  • Select own message and press "Edit message"
  • The composer should expand (issue 3.)
  • Press x to cancel (issues 1. 2.)

status: ready

@clauxx clauxx self-assigned this Nov 1, 2023
@status-im-auto
Copy link
Member

status-im-auto commented Nov 1, 2023

Jenkins Builds

Click to see older builds (42)
Commit #️⃣ Finished (UTC) Duration Platform Result
c6a7cb5 #1 2023-11-01 09:29:17 ~3 min tests 📄log
✔️ c6a7cb5 #1 2023-11-01 09:31:37 ~5 min android-e2e 🤖apk 📲
✔️ c6a7cb5 #1 2023-11-01 09:32:21 ~6 min android 🤖apk 📲
✔️ c6a7cb5 #1 2023-11-01 09:32:35 ~6 min ios 📱ipa 📲
✔️ f86fde2 #3 2023-11-02 21:41:44 ~5 min android 🤖apk 📲
✔️ f86fde2 #3 2023-11-02 21:43:27 ~7 min ios 📱ipa 📲
✔️ f86fde2 #3 2023-11-02 21:45:40 ~9 min android-e2e 🤖apk 📲
✔️ f86fde2 #3 2023-11-02 21:46:23 ~10 min tests 📄log
✔️ ec5c580 #4 2023-11-03 13:06:33 ~5 min android 🤖apk 📲
✔️ ec5c580 #4 2023-11-03 13:07:46 ~6 min android-e2e 🤖apk 📲
✔️ ec5c580 #4 2023-11-03 13:11:32 ~10 min ios 📱ipa 📲
✔️ ec5c580 #4 2023-11-03 13:11:45 ~10 min tests 📄log
✔️ 7d69fa9 #5 2023-11-03 15:07:06 ~6 min android 🤖apk 📲
✔️ 7d69fa9 #5 2023-11-03 15:10:03 ~9 min android-e2e 🤖apk 📲
✔️ 7d69fa9 #5 2023-11-03 15:10:23 ~10 min tests 📄log
✔️ 7d69fa9 #5 2023-11-03 15:10:42 ~10 min ios 📱ipa 📲
✔️ ba4d618 #6 2023-11-06 11:08:05 ~5 min android-e2e 🤖apk 📲
✔️ ba4d618 #6 2023-11-06 11:08:46 ~6 min android 🤖apk 📲
✔️ 7ad8165 #7 2023-11-06 11:17:41 ~7 min android 🤖apk 📲
✔️ 7ad8165 #7 2023-11-06 11:20:56 ~10 min android-e2e 🤖apk 📲
✔️ 7ad8165 #7 2023-11-06 11:23:48 ~13 min tests 📄log
✔️ 7ad8165 #7 2023-11-06 11:25:29 ~15 min ios 📱ipa 📲
✔️ 8c9c93a #8 2023-11-06 14:34:42 ~8 min android-e2e 🤖apk 📲
✔️ 8c9c93a #8 2023-11-06 14:35:24 ~8 min android 🤖apk 📲
✔️ 8c9c93a #8 2023-11-06 14:36:31 ~9 min tests 📄log
✔️ 8c9c93a #8 2023-11-06 14:38:41 ~12 min ios 📱ipa 📲
✔️ cb9ae2a #10 2023-11-07 13:22:29 ~5 min android-e2e 🤖apk 📲
✔️ cb9ae2a #10 2023-11-07 13:22:55 ~6 min android 🤖apk 📲
✔️ cb9ae2a #10 2023-11-07 13:25:14 ~8 min tests 📄log
✔️ cb9ae2a #10 2023-11-07 13:31:45 ~15 min ios 📱ipa 📲
a175f3d #11 2023-11-09 10:48:37 ~2 min tests 📄log
✔️ a175f3d #11 2023-11-09 10:51:39 ~5 min ios 📱ipa 📲
✔️ a175f3d #11 2023-11-09 10:52:55 ~6 min android-e2e 🤖apk 📲
✔️ a175f3d #11 2023-11-09 10:52:59 ~6 min android 🤖apk 📲
✔️ 2a3b4aa #12 2023-11-09 16:29:05 ~5 min ios 📱ipa 📲
✔️ 2a3b4aa #12 2023-11-09 16:29:50 ~6 min android-e2e 🤖apk 📲
✔️ 2a3b4aa #12 2023-11-09 16:29:57 ~6 min android 🤖apk 📲
✔️ 2a3b4aa #12 2023-11-09 16:33:16 ~9 min tests 📄log
✔️ 89e9eba #13 2023-11-09 17:53:31 ~5 min ios 📱ipa 📲
✔️ 89e9eba #13 2023-11-09 17:58:16 ~10 min android 🤖apk 📲
✔️ 89e9eba #13 2023-11-09 17:58:18 ~10 min tests 📄log
✔️ 89e9eba #13 2023-11-09 17:58:27 ~10 min android-e2e 🤖apk 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ b41760e #15 2023-11-16 14:06:25 ~5 min android 🤖apk 📲
✔️ b41760e #15 2023-11-16 14:06:27 ~5 min ios 📱ipa 📲
✔️ b41760e #15 2023-11-16 14:07:20 ~6 min android-e2e 🤖apk 📲
✔️ b41760e #15 2023-11-16 14:10:25 ~9 min tests 📄log
✔️ cdd3859 #16 2023-11-16 14:54:41 ~6 min ios 📱ipa 📲
✔️ cdd3859 #16 2023-11-16 14:58:09 ~9 min android-e2e 🤖apk 📲
✔️ cdd3859 #16 2023-11-16 14:58:25 ~9 min android 🤖apk 📲
✔️ cdd3859 #16 2023-11-16 14:59:21 ~10 min tests 📄log

@clauxx clauxx force-pushed the 17433-composer-expanded-on-cancel branch from c6a7cb5 to 99250ea Compare November 2, 2023 21:31
@clauxx clauxx marked this pull request as ready for review November 2, 2023 21:35
@OmarBasem
Copy link
Contributor

OmarBasem commented Nov 3, 2023

Hi @clauxx, thanks for your PR

I noticed an issue when trying to edit or reply, that the composer does not animate smoothly. Notice if the composer is blurred and then when trying to focus it animates smoothly. But when trying to edit a message the composer moves up suddenly

IMG_0.3.MOV

@clauxx
Copy link
Member Author

clauxx commented Nov 3, 2023

Hi @clauxx, thanks for your PR

I noticed an issue when trying to edit or reply, that the composer does not animate smoothly. Notice if the composer is blurred and then when trying to focus it animates smoothly. But when trying to edit a message the composer moves up suddenly

IMG_0.3.MOV

@OmarBasem Tried to smooth it out a bit, as it looked a bit worse when the input changed size, but the issue seems to not be related to this PR, as that was the case before it as well. This happens when focusing the input imperatively with the ref, as you can notice when replying to a message as well. We should have another issue for this, which would fix it everywhere we focus imperatively.

Also, added another fix to a bug I noticed, where the input had the wrong height after canceling edit.

@OmarBasem
Copy link
Contributor

We should have another issue for this, which would fix it everywhere we focus imperatively.

@clauxx is there an open issue for that?

@clauxx
Copy link
Member Author

clauxx commented Nov 6, 2023

@OmarBasem Now it seems to animate fine 🤷

Simulator.Screen.Recording.-.iPhone.11.Pro.-.2023-11-06.at.13.03.24.mp4

@clauxx
Copy link
Member Author

clauxx commented Nov 6, 2023

Also opened two other issues I noticed while fixing this one #17827 and #17825

@OmarBasem
Copy link
Contributor

@OmarBasem Now it seems to animate fine 🤷

Okay good :)

Good work 🚀

@status-im-auto
Copy link
Member

78% of end-end tests have passed

Total executed tests: 45
Failed tests: 6
Expected to fail tests: 4
Passed tests: 35
IDs of failed tests: 702777,703133,702742,702869,702786,702846 
IDs of expected to fail tests: 702732,702783,702731,702808 

Failed tests (6)

Click to expand
  • Rerun failed tests

  • Class TestCommunityMultipleDeviceMergedTwo:

    1. test_community_mentions_push_notification, id: 702786

    Device 2: Getting PN by 'user_2'
    Device 2: Looking for a message by text: user_2

    critical/chats/test_public_chat_browsing.py:902: in test_community_mentions_push_notification
        self.errors.verify_no_errors()
    base_test_case.py:191: in verify_no_errors
        pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
     Can not edit a message with a mention
    E    Edited message is not shown correctly for the (receiver) admin
    



    Device sessions

    Class TestCommunityOneDeviceMerged:

    1. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133

    # STEP: Check that can login with different user
    Device 1: Find Button by accessibility id: show-profiles

    critical/chats/test_public_chat_browsing.py:251: in test_restore_multiaccount_with_waku_backup_remove_switch
        self.sign_in.show_profiles_button.click()
    ../views/base_element.py:90: in click
        self.find_element().click()
    ../views/base_element.py:79: in find_element
        raise NoSuchElementException(
     Device 1: Button by accessibility id: `show-profiles` is not found on the screen; For documentation on this error, please visit: https://www.selenium.dev/documentation/webdriver/troubleshooting/errors#no-such-element-exception
    



    Device sessions

    2. test_community_copy_and_paste_message_in_chat_input, id: 702742

    Device 1: ChatMessageInput element not found
    Device 1: Sending message 'mmmeowesage_text'

    critical/chats/test_public_chat_browsing.py:91: in test_community_copy_and_paste_message_in_chat_input
        self.channel.send_message(message)
    ../views/chat_view.py:1003: in send_message
        self.chat_message_input.wait_for_element(wait_chat_input_sec)
    ../views/base_element.py:121: in wait_for_element
        raise TimeoutException(
     Device `1`: `ChatMessageInput` by` accessibility id`: `chat-message-input` is not found on the screen after wait_for_element
    



    Device sessions

    3. test_community_undo_delete_message, id: 702869

    Device 1: ChatMessageInput element not found
    Device 1: Sending message 'message to delete and undo'

    critical/chats/test_public_chat_browsing.py:106: in test_community_undo_delete_message
        self.channel.send_message(message_to_delete)
    ../views/chat_view.py:1003: in send_message
        self.chat_message_input.wait_for_element(wait_chat_input_sec)
    ../views/base_element.py:121: in wait_for_element
        raise TimeoutException(
     Device `1`: `ChatMessageInput` by` accessibility id`: `chat-message-input` is not found on the screen after wait_for_element
    



    Device sessions

    4. test_community_navigate_to_channel_when_relaunch, id: 702846

    Device 1: ChatMessageInput element not found
    Device 1: Sending message 'some_text'

    critical/chats/test_public_chat_browsing.py:77: in test_community_navigate_to_channel_when_relaunch
        self.channel.send_message(text_message)
    ../views/chat_view.py:1003: in send_message
        self.chat_message_input.wait_for_element(wait_chat_input_sec)
    ../views/base_element.py:121: in wait_for_element
        raise TimeoutException(
     Device `1`: `ChatMessageInput` by` accessibility id`: `chat-message-input` is not found on the screen after wait_for_element
    



    Device sessions

    Class TestActivityCenterContactRequestMultipleDevicePR:

    1. test_add_contact_field_validation, id: 702777

    ## Creating new multiaccount (password:'qwerty1234', keycard:'False', enable_notification: 'False')
    Device 2: Wait for element Button for max 20s and click when it is available

    activity_center/test_activity_center.py:147: in test_add_contact_field_validation
        self.device_2.create_user(username=new_username_2, user_number=self.device_2_users_number)
    ../views/sign_in_view.py:230: in create_user
        self.show_profiles_button.wait_and_click(20)
    ../views/base_element.py:96: in wait_and_click
        self.wait_for_visibility_of_element(sec)
    ../views/base_element.py:139: in wait_for_visibility_of_element
        raise TimeoutException(
     Device 2: Button by accessibility id:`show-profiles` is not found on the screen after wait_for_visibility_of_element
    



    Device sessions

    Expected to fail tests (4)

    Click to expand

    Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:

    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:617: in test_1_1_chat_is_shown_message_sent_delivered_from_offline
        self.errors.verify_no_errors()
    base_test_case.py:191: 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! 
    

    [[Data delivery issue]]

    Device sessions

    Class TestOneToOneChatMultipleSharedDevicesNewUi:

    1. 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]]

    2. test_group_chat_offline_pn, id: 702808

    Device 3: Looking for a message by text: message from old member
    Device 3: Looking for a message by text: message from new member

    critical/chats/test_group_chat.py:323: in test_group_chat_offline_pn
        self.errors.verify_no_errors()
    base_test_case.py:191: in verify_no_errors
        pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
     Messages PN was not fetched from offline 
    

    [[Data delivery issue]]

    Device sessions

    Passed tests (35)

    Click to expand

    Class TestCommunityMultipleDeviceMergedTwo:

    1. test_community_markdown_support, id: 702809
    Device sessions

    2. test_community_hashtag_links_to_community_channels, id: 702948
    Device sessions

    3. test_community_leave, id: 702845
    Device sessions

    4. test_community_join_when_node_owner_offline, id: 703629
    Device sessions

    Class TestCommunityOneDeviceMerged:

    1. test_community_discovery, id: 703503
    Device sessions

    2. test_community_mute_community_and_channel, id: 703382
    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_push_emoji, id: 702813
    Device sessions

    4. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
    Device sessions

    5. test_1_1_chat_edit_message, id: 702855
    Device sessions

    6. test_1_1_chat_send_image_save_and_share, id: 703391
    Device sessions

    7. test_1_1_chat_message_reaction, id: 702730
    Device sessions

    Class TestGroupChatMultipleDeviceMergedNewUI:

    1. test_group_chat_mute_chat, id: 703495
    Device sessions

    2. test_group_chat_send_image_save_and_share, id: 703297
    Device sessions

    3. test_group_chat_reactions, id: 703202
    Device sessions

    4. test_group_chat_join_send_text_messages_push, id: 702807
    Device sessions

    Class TestActivityMultipleDevicePRTwo:

    1. test_activity_center_mentions, id: 702957
    Device sessions

    2. test_activity_center_admin_notification_accept_swipe, id: 702958
    Device sessions

    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_navigation_jump_to, id: 702936
    Device sessions

    2. test_activity_center_reply_read_unread_delete_filter_swipe, id: 702947
    Device sessions

    Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:

    1. test_1_1_chat_delete_via_long_press_relogin, id: 702784
    Device sessions

    2. test_1_1_chat_mute_chat, id: 703496
    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_message_delete, id: 702839
    Device sessions

    7. test_community_message_send_check_timestamps_sender_username, id: 702838
    Device sessions

    8. test_community_links_with_previews_github_youtube_twitter_gif_send_enable, id: 702844
    Device sessions

    9. test_community_message_edit, id: 702843
    Device sessions

    10. test_community_unread_messages_badge, id: 702841
    Device sessions

    @churik
    Copy link
    Member

    churik commented Nov 15, 2023

    Found one regression issue that wasn't captured before:

    ISSUE 6: edited message content in composer is reset when you re-open the chat

    Steps:

    1. Send message
    2. Start to edit it
    3. Go back and reopen the chat

    Expected result:
    Text in composer is unchanged

    Actual result:
    the text gets back to the initial message content

    FILE.2023-11-15.17.52.01.mp4

    OS: IOS, Android

    The issue is not reproducible with the current nightly.
    Video from nightly build:

    FILE.2023-11-15.17.51.29.mp4

    @clauxx clauxx force-pushed the 17433-composer-expanded-on-cancel branch from 89e9eba to 97e7d46 Compare November 16, 2023 13:59
    @clauxx
    Copy link
    Member Author

    clauxx commented Nov 16, 2023

    @churik @pavloburykh @OmarBasem

    Fixed issue 6 and (hopefully) issue 1 is also fixed. It's a bit difficult to reproduce cause it was happening kinda randomly, but couldn't reproduce it at all after the fix.

    Additionally, noticed there was an issue with hiding the keyboard when closing a reply, similar to what we had here. Dealt with it here cause it was a quick fix.

    @ibrkhalil There was a release blocker issue here for which I needed changes from your PR, so I merged it here to make the QA process quicker. Hope that's fine by you ☺️.

    And lastly, remembered that this PR also fixed another composer issue: #17361

    @churik
    Copy link
    Member

    churik commented Nov 16, 2023

    @clauxx @ibrkhalil
    thank you for work.
    I believe the composer's behavior now became a way more reliable, but #17361 is not fixed there and regression about ISSUE 1 is reproducible for me.

    #17361 video:

    FILE.2023-11-16.17.42.12.mp4

    ISSUE 1 video:

    FILE.2023-11-16.17.43.22.mp4

    both issues are very nice to fix, but not critical for PR. I can create them separately and we'll fix it in the next release, what do you think?

    @clauxx
    Copy link
    Member Author

    clauxx commented Nov 16, 2023

    @churik yepp, i guess i musunderstood the first one with the composer height. The issue 1 is weird, i think it makes sense to fix them separately. Can you assign them to me please?

    @churik
    Copy link
    Member

    churik commented Nov 16, 2023

    Please, cherry-pick your commit in release branch too.
    Thank you, I created #17929 and assigned it to you

    @clauxx clauxx merged commit 8f8c8de into develop Nov 17, 2023
    6 checks passed
    @clauxx clauxx deleted the 17433-composer-expanded-on-cancel branch November 17, 2023 09:32
    @churik
    Copy link
    Member

    churik commented Nov 17, 2023

    @clauxx can you cherry-pick it to release branch?

    @clauxx
    Copy link
    Member Author

    clauxx commented Nov 17, 2023

    @churik on it. Have some issue with my GPG keys that I'm trying to fix since an hour, so can't push the cherry-picked commit. Will check with Andrea if he can do the cherrypicking on his side.

    cammellos pushed a commit that referenced this pull request Nov 17, 2023
    * fix: composer height when entering and canceling edit
    
    * fix: blur the composer input when canceling edit
    
    * fix: focusing animation and composer height after blur
    
    * fix: input height when canceling edit while unfocused
    
    * ref: removed arbitrary keyboard check
    
    * fix: moved edit-mentions logic to use-edit to fix unresolved mention
    
    * fix: composer edit should put the cursor at the end
    
    * fix: (potentially) fixing the mention not resolved during edit
    
    * fix: emoji-kb handler changing the height when default kb appears
    
    * Fix text content when editing and reentering chat
    
    * prevent composer when focusing on opening chat with edit/reply
    
    * clean
    
    * Clauxx comments
    
    * Apply for reply
    
    * Lintil soup = yummy
    
    * refactor variable name
    
    * Extract the focusing logic from the data setting logic
    
    * Edge case
    
    * fix: composer mention key & edit re-enter issues
    
    * fix: reply cancel input blur and smooth reply focus
    
    ---------
    
    Co-authored-by: Ibrkhalil <vampirekid017@gmail.com>
    @clauxx
    Copy link
    Member Author

    clauxx commented Nov 17, 2023

    @churik it's done!

    yevh-berdnyk pushed a commit that referenced this pull request Dec 8, 2023
    * fix: composer height when entering and canceling edit
    
    * fix: blur the composer input when canceling edit
    
    * fix: focusing animation and composer height after blur
    
    * fix: input height when canceling edit while unfocused
    
    * ref: removed arbitrary keyboard check
    
    * fix: moved edit-mentions logic to use-edit to fix unresolved mention
    
    * fix: composer edit should put the cursor at the end
    
    * fix: (potentially) fixing the mention not resolved during edit
    
    * fix: emoji-kb handler changing the height when default kb appears
    
    * Fix text content when editing and reentering chat
    
    * prevent composer when focusing on opening chat with edit/reply
    
    * clean
    
    * Clauxx comments
    
    * Apply for reply
    
    * Lintil soup = yummy
    
    * refactor variable name
    
    * Extract the focusing logic from the data setting logic
    
    * Edge case
    
    * fix: composer mention key & edit re-enter issues
    
    * fix: reply cancel input blur and smooth reply focus
    
    ---------
    
    Co-authored-by: Ibrkhalil <vampirekid017@gmail.com>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    Archived in project
    Archived in project
    7 participants