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.

Sign up for GitHub

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

Jump to bottom

[#16278] fix tips in white border #16716

Merged
merged 4 commits into from
Jul 26, 2023
Merged

[#16278] fix tips in white border #16716

merged 4 commits into from
Jul 26, 2023

Conversation

ulisesmac
Copy link
Contributor

@ulisesmac ulisesmac commented Jul 18, 2023 •
edited
Loading

Partial solution to #16278

Summary

This PR fixes the tips in the white border
image

Review notes

Sometimes in React Native is hard to perfectly center an UI element due to the unit managed in different devices, the tip in these borders had that problem. I solved it by reducing the width from 1 to 0.9 and it worked in the devices I tested (emulators and physical devices).

Platforms

  • Android
  • iOS

Steps to test

  • Open Status in a fresh install and click on Sign in, or add a new existing Status profile
  • Activate the camera
  • The border tips are not outside the view

status: ready

@ulisesmac ulisesmac self-assigned this Jul 18, 2023
@status-im-auto
Copy link
Member

status-im-auto commented Jul 18, 2023 •
edited
Loading

Jenkins Builds

Click to see older builds (11)
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 205585d #1 2023-07-18 19:32:29 ~7 min android-e2e 🤖apk 📲
✔️ 205585d #1 2023-07-18 19:32:33 ~7 min android 🤖apk 📲
✔️ 205585d #1 2023-07-18 19:33:33 ~8 min tests 📄log
✔️ da99bf2 #2 2023-07-20 22:32:17 ~6 min android 🤖apk 📲
✔️ da99bf2 #2 2023-07-20 22:32:25 ~6 min android-e2e 🤖apk 📲
✔️ da99bf2 #2 2023-07-20 22:33:58 ~7 min ios 📱ipa 📲
✔️ da99bf2 #2 2023-07-20 22:34:08 ~8 min tests 📄log
✔️ 95c21e5 #3 2023-07-21 20:22:06 ~5 min android-e2e 🤖apk 📲
✔️ 95c21e5 #3 2023-07-21 20:23:15 ~6 min ios 📱ipa 📲
✔️ 95c21e5 #3 2023-07-21 20:24:26 ~7 min android 🤖apk 📲
✔️ 95c21e5 #3 2023-07-21 20:25:24 ~8 min tests 📄log
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 39ac25c #4 2023-07-26 07:17:42 ~6 min android-e2e 🤖apk 📲
✔️ 39ac25c #4 2023-07-26 07:17:49 ~6 min android 🤖apk 📲
✔️ 39ac25c #4 2023-07-26 07:18:04 ~6 min ios 📱ipa 📲
✔️ 39ac25c #4 2023-07-26 07:19:41 ~8 min tests 📄log
✔️ 07bfb17 #5 2023-07-26 20:03:58 ~6 min ios 📱ipa 📲
✔️ 07bfb17 #5 2023-07-26 20:04:05 ~6 min android 🤖apk 📲
✔️ 07bfb17 #5 2023-07-26 20:04:08 ~7 min android-e2e 🤖apk 📲
✔️ 07bfb17 #5 2023-07-26 20:05:05 ~7 min tests 📄log

ibrkhalil
ibrkhalil reviewed Jul 18, 2023
View reviewed changes
src/status_im2/contexts/syncing/scan_sync_code/view.cljs
[rn/view {:style (style/border border1 border2 corner)}])
(defn- white-border
[corner]
(let [border-styles (style/white-border corner)]
Copy link
Contributor

Choose a reason for hiding this comment

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

White border is a variable, Why use it as a function?

src/status_im2/contexts/syncing/scan_sync_code/view.cljs Show resolved Hide resolved
src/status_im2/contexts/syncing/scan_sync_code/view.cljs Show resolved Hide resolved
ibrkhalil
ibrkhalil approved these changes Jul 19, 2023
View reviewed changes
Copy link
Contributor

@ibrkhalil ibrkhalil left a comment

Choose a reason for hiding this comment

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

lgtm

@ulisesmac ulisesmac force-pushed the 16278-fix-tips-in-white-border branch from 205585d to da99bf2 Compare July 20, 2023 22:25
@ulisesmac ulisesmac force-pushed the 16278-fix-tips-in-white-border branch from da99bf2 to 95c21e5 Compare July 21, 2023 20:16
@status-im-auto
Copy link
Member

81% of end-end tests have passed

Total executed tests: 36
Failed tests: 7
Passed tests: 29
Not executed tests: 3

IDs of not executed tests: 703496,703495,703382 

IDs of failed tests: 702732,703133,702957,702851,703202,702839,702731

Not executed tests (3)

Click to expand
  • Rerun not executed tests

    • Failed tests (7)

    Click to expand
  • Rerun failed tests

    • 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_reactions, id: 703202

    Device 3: Adding one more 'thumbs-up' reaction or removing an added one
    Device 3: Find Button by accessibility id: emoji-reaction-2 

    critical/chats/test_group_chat.py:244: in test_group_chat_reactions
    self.chats[2].add_remove_same_reaction(message=message, emoji="thumbs-up")
../views/chat_view.py:1032: in add_remove_same_reaction
    element.click()
../views/base_element.py:91: in click
    self.find_element().click()
../views/base_element.py:80: in find_element
    raise NoSuchElementException(
 Device 3: Button by accessibility id: `emoji-reaction-2` is not found on the screen



    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 TestCommunityMultipleDeviceMerged:

    1. test_community_message_delete, id: 702839

    Device 2: Looking for a message by text: delete for me
    Device 1: Looking for a message by text: delete for me 

    critical/test_public_chat_browsing.py:541: in test_community_message_delete
    self.errors.verify_no_errors()
base_test_case.py:183: in verify_no_errors
    pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
 Deleted message is shown in channel



    Device sessions

    Class TestActivityMultipleDevicePR:

    1. test_activity_center_mentions, id: 702957

    # STEP: Tap on it and check redirect to channel
    Device 1: Looking for a message by text: user1 

    None; 
 RemoteDisconnected



    Device sessions

    Class TestCommunityOneDeviceMerged:

    1. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133

    Device 1: Find Button by accessibility id: show-profiles
    Device 1: Tap on found: Button 

    critical/test_public_chat_browsing.py:433: in test_restore_multiaccount_with_waku_backup_remove_switch
    self.errors.verify_no_errors()
base_test_case.py:183: in verify_no_errors
    pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
 Contact(s) was (were) not restored from backup: Test_contact, MyCustomNickname!



    Device sessions

    Class TestActivityCenterContactRequestMultipleDevicePR:

    1. test_activity_center_contact_request_accept_swipe_mark_all_as_read, id: 702851

    Device 2: Find Button by accessibility id: tab-contacts
    Device 2: Tap on found: Button 

    medium/test_activity_center.py:111: in test_activity_center_contact_request_accept_swipe_mark_all_as_read
    self.errors.verify_no_errors()
base_test_case.py:183: in verify_no_errors
    pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
 Contact was not added to contact list after accepting contact request (as receiver)
E    Contact was not added to contact list after accepting contact request (as sender)



    Device sessions

    Passed tests (29)

    Click to expand

    Class TestGroupChatMultipleDeviceMergedNewUI:

    1. test_group_chat_send_image_save_and_share, id: 703297
    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 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_is_shown_message_sent_delivered_from_offline, id: 702783
    Device sessions

    7. test_1_1_chat_edit_message, id: 702855
    Device sessions

    8. test_1_1_chat_send_image_save_and_share, id: 703391
    Device sessions

    9. test_1_1_chat_message_reaction, id: 702730
    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_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_leave, id: 702845
    Device sessions

    11. test_community_unread_messages_badge, id: 702841
    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

    3. test_activity_center_admin_notification_accept_swipe, id: 702958
    Device sessions

    Class TestCommunityOneDeviceMerged:

    1. test_community_copy_and_paste_message_in_chat_input, id: 702742
    Device sessions

    2. test_community_navigate_to_channel_when_relaunch, id: 702846
    Device sessions

    Class TestActivityCenterContactRequestMultipleDevicePR:

    1. test_activity_center_contact_request_decline, id: 702850
    Device sessions

    @qoqobolo qoqobolo self-assigned this Jul 24, 2023
    @status-im-auto
    Copy link
    Member

    57% of end-end tests have passed

    Total executed tests: 7
Failed tests: 3
Passed tests: 4

    IDs of failed tests: 702731,703133,702732

    Failed tests (3)

    Click to expand
  • Rerun failed tests

    • Class TestGroupChatMultipleDeviceMergedNewUI:

    1. test_group_chat_pin_messages, id: 702732 
    Test is not run, e2e blocker

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

    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 TestCommunityOneDeviceMerged:

    1. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133

    Device 1: Find Button by accessibility id: show-profiles
    Device 1: Tap on found: Button 

    critical/test_public_chat_browsing.py:433: in test_restore_multiaccount_with_waku_backup_remove_switch
    self.errors.verify_no_errors()
base_test_case.py:183: in verify_no_errors
    pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
 Contact(s) was (were) not restored from backup: Test_contact, MyCustomNickname!



    Device sessions

    Passed tests (4)

    Click to expand

    Class TestGroupChatMultipleDeviceMergedNewUI:

    1. test_group_chat_reactions, id: 703202
    Device sessions

    Class TestActivityCenterContactRequestMultipleDevicePR:

    1. test_activity_center_contact_request_accept_swipe_mark_all_as_read, id: 702851
    Device sessions

    Class TestActivityMultipleDevicePR:

    1. test_activity_center_mentions, id: 702957
    Device sessions

    Class TestCommunityMultipleDeviceMerged:

    1. test_community_message_delete, id: 702839
    Device sessions

    @qoqobolo
    Copy link
    Contributor

    Hey @ulisesmac , thanks for your work!

    The tip looks good now, but the frame placement is still different from the design.

    Design:

    Screenshot 2023-07-24 at 16 00 47

    PR:

    Screenshot 2023-07-24 at 16 02 20

    In the description, you indicate that this PR fixes #16278, but in fact only partially. Could you clarify please, the second part of the problem will be fixed separately, right?
    In this case, this PR can be merged, but please make sure that the issue is not closed.

    @ulisesmac ulisesmac force-pushed the 16278-fix-tips-in-white-border branch from 95c21e5 to 39ac25c Compare July 25, 2023 23:11
    @ulisesmac
    Copy link
    Contributor Author

    @qoqobolo
    The problem is device's DPI, when we add an unit to paddings or margins, they are not always moved equally in all devices, so, these borders are shown correctly in some devices.
    We have this problem in many components but it's not always noticeable.

    I will not close this issue, and I will reference this comment.

    @qoqobolo
    Copy link
    Contributor

    @ulisesmac gotcha!
    Thanks for the explanation and the fix. Let's merge it!

    @ulisesmac ulisesmac force-pushed the 16278-fix-tips-in-white-border branch from 39ac25c to 07bfb17 Compare July 26, 2023 19:56
    @ulisesmac ulisesmac merged commit 4f04476 into develop Jul 26, 2023
    2 checks passed
    @ulisesmac ulisesmac deleted the 16278-fix-tips-in-white-border branch July 26, 2023 20:08
    @ulisesmac ulisesmac mentioned this pull request Jul 26, 2023
    Closed
    andresceballosm pushed a commit to andresceballosm/status-mobile that referenced this pull request Aug 6, 2023
    @ulisesmac @andresceballosm
    [status-im#16278] fix tips in white camera border (status-im#16716)
    236c7fa 
    * Remove unnecessary wrapper
* Fix border tips and refactor
* Add comment about using 1.9 width
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers

    @ibrkhalil ibrkhalil ibrkhalil approved these changes

    @J-Son89 J-Son89 J-Son89 approved these changes

    @smohamedjavid smohamedjavid smohamedjavid approved these changes

    Assignees

    @qoqobolo qoqobolo

    @ulisesmac ulisesmac

    Labels
    request-manual-qa Tested - OK
    Projects
    No open projects
    Legacy QA Automation Project
    Archived in project
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.
    6 participants
    @ulisesmac @status-im-auto @qoqobolo @smohamedjavid @J-Son89 @ibrkhalil