App locked screen - design review changes #15927

ajayesivan · 2023-05-17T03:06:06Z

Partial fix for #15788

Figma: https://www.figma.com/file/o4qG1bnFyuyFOvHQVGgeFY/Onboarding-for-Mobile?type=design&node-id=2941-331293&t=vV2J6OGE4Ebma1GS-4

Fixes

Forgot password design update
Login button design update
Wrong password error design update
Keyboard Appearance fix for iOS - Now dark mode screens will have the dark-themed keyboard on iOS. (keyboard appearance option is not available on Android)

Related Design review comments: https://www.figma.com/file/Tf5nfkYvpbnNCo4rKLK7lS/Feedback-for-Mobile?type=design&node-id=1946-147125&t=veHPm94UUhGk04v3-4

Areas to test

App locked screen
Onboarding flow - All onboarding input should have the dark-themed keyboard on iOS.
Inputs in general on iOS - All dark-themed inputs should have dark-themed keyboards on iOS.

status: ready

status-im-auto · 2023-05-17T03:12:05Z

Jenkins Builds

Click to see older builds (21)

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`1c80cce`	#1	2023-05-17 03:12:04	~5 min	`android`	🤖`apk` 📲
✔️	`1c80cce`	#1	2023-05-17 03:12:39	~6 min	`android-e2e`	🤖`apk` 📲
✔️	`1c80cce`	#1	2023-05-17 03:13:09	~6 min	`ios`	📱`ipa` 📲
✔️	`1c80cce`	#1	2023-05-17 03:13:12	~6 min	`tests`	📄`log`

❌	`64f554a`	#2	2023-05-17 23:05:05	~3 min	`tests`	📄`log`
✔️	`64f554a`	#2	2023-05-17 23:07:15	~5 min	`android-e2e`	🤖`apk` 📲
✔️	`64f554a`	#2	2023-05-17 23:07:23	~5 min	`android`	🤖`apk` 📲
✔️	`64f554a`	#2	2023-05-17 23:07:47	~6 min	`ios`	📱`ipa` 📲

❌	`72150ab`	#3	2023-05-17 23:33:02	~3 min	`tests`	📄`log`

❌	`4fe323f`	#4	2023-05-17 23:38:16	~4 min	`tests`	📄`log`
✔️	`4fe323f`	#4	2023-05-17 23:38:49	~5 min	`android-e2e`	🤖`apk` 📲
✔️	`4fe323f`	#4	2023-05-17 23:38:55	~5 min	`android`	🤖`apk` 📲
✔️	`4fe323f`	#4	2023-05-17 23:39:49	~6 min	`ios`	📱`ipa` 📲

❌	`e6022d8`	#6	2023-05-18 21:55:09	~4 min	`tests`	📄`log`
✔️	`e6022d8`	#6	2023-05-18 21:56:28	~5 min	`android-e2e`	🤖`apk` 📲
✔️	`e6022d8`	#6	2023-05-18 21:57:11	~6 min	`ios`	📱`ipa` 📲
✔️	`e6022d8`	#6	2023-05-18 21:58:26	~7 min	`android`	🤖`apk` 📲

❌	`4706824`	#8	2023-05-19 10:07:14	~3 min	`tests`	📄`log`
✔️	`4706824`	#8	2023-05-19 10:09:32	~5 min	`android-e2e`	🤖`apk` 📲
✔️	`4706824`	#8	2023-05-19 10:09:52	~5 min	`android`	🤖`apk` 📲
✔️	`4706824`	#8	2023-05-19 10:11:12	~7 min	`ios`	📱`ipa` 📲

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`c993e4b`	#10	2023-05-19 11:44:11	~6 min	`android-e2e`	🤖`apk` 📲
✔️	`c993e4b`	#10	2023-05-19 11:44:41	~7 min	`tests`	📄`log`
✔️	`c993e4b`	#10	2023-05-19 11:44:41	~7 min	`ios`	📱`ipa` 📲
✔️	`c993e4b`	#10	2023-05-19 11:45:27	~7 min	`android`	🤖`apk` 📲

✔️	`771d497`	#11	2023-05-19 11:58:42	~6 min	`ios`	📱`ipa` 📲
✔️	`771d497`	#11	2023-05-19 12:01:38	~9 min	`android-e2e`	🤖`apk` 📲
✔️	`771d497`	#11	2023-05-19 12:01:44	~9 min	`android`	🤖`apk` 📲
✔️	`771d497`	#11	2023-05-19 12:03:56	~12 min	`tests`	📄`log`

Parveshdhull · 2023-05-17T04:30:18Z

src/quo2/components/inputs/input/view.cljs

@@ -100,6 +101,7 @@
           (cond-> {:style                  (style/input colors-by-status small? @multiple-lines?)
                    :accessibility-label    :input
                    :placeholder-text-color (:placeholder colors-by-status)
+                    :keyboard-appearance    (colors/theme-colors :light :dark override-theme)


hi, :light and :dark are not colors, Please use (if (colors/dark?) :dark :light)

Or maybe we should rename function (or create a new one) and put in utils etc.
Something like (utils/theme-value light-theme-value dark-theme-value override-theme)
@J-Son89 @ilmotta wdyt?

or put in quo2.theme?

cc: @flexsurfer

I have seen the function being used rarely for things other than color, like here:

status-mobile/src/status_im2/contexts/chat/composer/style.cljs

Line 12 in 0c16ffc

:shadow-opacity (colors/theme-colors 0.1 0.7)

.

Despite the name, the function returns whatever we pass in the props based on the theme/override-theme. But I think renaming/making a new function can make more sense.

What about a function/macro called if-light-theme?
Not the best name, but I've seen in this PR it'd be used a lot. IMO using theme-colors for returning not color values is not a good idea.

I agree with @ulisesmac and @Parveshdhull, it's not a good idea to use this function even if it works here. It's intention is to set colors. Let's create some other helper as suggested, perhaps something more generic in name 👍

I agree with @J-Son89 and others. And I also like @Parveshdhull's suggestion for a utility function outside the colors ns to get any value based on a theme.

I agree with you guys. I will create a generic utility function and update the PR.

@Parveshdhull @ulisesmac @J-Son89 @ilmotta I have added a theme/theme-value function. Basically, it's the same as colors/theme-colors function with a different name. Let me know what you think of this implementation.

ulisesmac

Looks good 👍 thanks for your PR!

I left some small comments

src/quo2/components/buttons/button.cljs

src/status_im2/contexts/onboarding/profiles/view.cljs

src/status_im2/contexts/onboarding/profiles/style.cljs

ulisesmac · 2023-05-17T07:50:01Z

src/quo2/components/inputs/input/view.cljs

@@ -100,6 +101,7 @@
           (cond-> {:style                  (style/input colors-by-status small? @multiple-lines?)
                    :accessibility-label    :input
                    :placeholder-text-color (:placeholder colors-by-status)
+                    :keyboard-appearance    (colors/theme-colors :light :dark override-theme)


What about a function/macro called if-light-theme?
Not the best name, but I've seen in this PR it'd be used a lot. IMO using theme-colors for returning not color values is not a good idea.

ajayesivan · 2023-05-17T23:36:34Z

@ulisesmac I have updated the PR with all your suggestions.

ilmotta · 2023-05-18T09:35:19Z

src/quo2/theme.cljs

+  ([light-value dark-value override-theme]
+   (let [theme (or override-theme (get-theme))]
+     (if (= theme :light) light-value dark-value))))


Tiny one: just missing the newline character on line 24.

Done. Missed configuring that in my new editor.

status-im-auto · 2023-05-18T23:09:35Z

94% of end-end tests have passed

Total executed tests: 32
Failed tests: 2
Passed tests: 30

IDs of failed tests: 702844,702838

Failed tests (2)

Click to expand

Rerun failed tests

Class TestCommunityMultipleDeviceMerged:

1. test_community_links_with_previews_github_youtube_twitter_gif_send_enable, id: 702844
Device 2: Tap on found: SendMessageButton Device 2: Find `Button` by `xpath`: `//[@text="Don't ask me again"]` critical/test_public_chat_browsing.py:604: in test_community_links_with_previews_github_youtube_twitter_gif_send_enable self.channel_2.element_by_translation_id("dont-ask").click() ../views/base_element.py:91: in click self.find_element().click() ../views/base_element.py:80: in find_element raise NoSuchElementException( Device 2: Button by xpath: `//[@text="Don't ask me again"]` is not found on the screen Device sessions Device 1: Steps, video, logs Failure screenshot Device 2: Steps, video, logs Failure screenshot
2. test_community_message_send_check_timestamps_sender_username, id: 702838
Device 2: Verifying that 'hello' is under today Device 2: Looking for a message by text: hello critical/test_public_chat_browsing.py:456: in test_community_message_send_check_timestamps_sender_username channel.verify_message_is_under_today_text(message, self.errors) ../views/chat_view.py:935: in verify_message_is_under_today_text message_element.wait_for_visibility_of_element() ../views/base_element.py:135: in wait_for_visibility_of_element raise TimeoutException( Device 2: ChatElementByText by xpath:`//[starts-with(@text,'hello')]/ancestor::android.view.ViewGroup[@content-desc='chat-item']` is not found on the screen after wait_for_visibility_of_element [[blocked by 14797]]* Device sessions Device 1: Steps, video, logs Failure screenshot Device 2: Steps, video, logs Failure screenshot

Passed tests (30)

Click to expand

Class TestActivityCenterContactRequestMultipleDevicePR:

1. test_activity_center_contact_request_decline, id: 702850
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
2. test_activity_center_contact_request_accept_swipe_mark_all_as_read, id: 702851
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs

Class TestGroupChatMultipleDeviceMergedNewUI:

1. test_group_chat_pin_messages, id: 702732
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs Device 3: Steps, video, logs
2. test_group_chat_join_send_text_messages_push, id: 702807
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs Device 3: Steps, video, logs
3. test_group_chat_offline_pn, id: 702808
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs Device 3: Steps, video, logs

Class TestActivityMultipleDevicePR:

1. test_activity_center_reply_read_unread_delete_filter_swipe, id: 702947
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
2. test_activity_center_admin_notification_accept_swipe, id: 702958
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
3. test_navigation_jump_to, id: 702936
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
4. test_activity_center_mentions, id: 702957
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs

Class TestOneToOneChatMultipleSharedDevicesNewUi:

1. test_1_1_chat_text_message_delete_push_disappear, id: 702733
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
2. test_1_1_chat_edit_message, id: 702855
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
3. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
4. test_1_1_chat_message_reaction, id: 702730
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
5. test_1_1_chat_emoji_send_reply_and_open_link, id: 702782
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
6. test_1_1_chat_is_shown_message_sent_delivered_from_offline, id: 702783
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
7. test_1_1_chat_pin_messages, id: 702731
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
8. test_1_1_chat_push_emoji, id: 702813
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
9. test_1_1_chat_delete_via_long_press_relogin, id: 702784
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs

Class TestCommunityOneDeviceMerged:

1. test_community_navigate_to_channel_when_relaunch, id: 702846
Device sessions Device 1: Steps, video, logs
2. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133
Device sessions Device 1: Steps, video, logs
3. test_community_copy_and_paste_message_in_chat_input, id: 702742
Device sessions Device 1: Steps, video, logs

Class TestCommunityMultipleDeviceMerged:

1. test_community_emoji_send_copy_paste_reply, id: 702840
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
2. test_1_1_chat_image_send_reply, id: 702859
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
3. test_community_mentions_push_notification, id: 702786
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
4. test_community_contact_block_unblock_offline, id: 702894
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
5. test_community_leave, id: 702845
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
6. test_community_unread_messages_badge, id: 702841
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
7. test_community_message_delete, id: 702839
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
8. test_community_mark_all_messages_as_read, id: 703086
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
9. test_community_message_edit, id: 702843
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs

VolodLytvynenko · 2023-05-19T08:22:28Z

Hi, @ajayesivan integration tests failed https://ci.status.im/job/status-mobile/job/prs/job/tests/job/PR-15927/6/consoleText. Could you check it, please?

ajayesivan · 2023-05-19T10:05:45Z

Hi, @ajayesivan integration tests failed https://ci.status.im/job/status-mobile/job/prs/job/tests/job/PR-15927/6/consoleText. Could you check it, please?

@VladimrLitvinenko I ran a make test that passed without any error. Are there any other tests? How can I reproduce the issue you shared locally?

VolodLytvynenko · 2023-05-19T11:59:45Z

@ajayesivan Thanks for the PR. No issues from my side. Ready to be merged

ajayesivan · 2023-05-26T13:35:29Z

src/status_im2/contexts/onboarding/profiles/view.cljs

@@ -178,7 +180,8 @@
        sign-in-enabled?     (rf/sub [:sign-in-enabled?])
        profile-picture      (:uri (first (:images multiaccount)))]
    [rn/keyboard-avoiding-view
-     {:style style/login-container}
+     {:style                  style/login-container
+      :keyboardVerticalOffset (- (safe-area/get-bottom))}


@smohamedjavid Here

status-github-bot bot added this to REVIEW in Pipeline for QA May 17, 2023

ajayesivan self-assigned this May 17, 2023

ajayesivan requested review from rahulpsingh, yqrashawn, Parveshdhull and ulisesmac May 17, 2023 03:08

ajayesivan mentioned this pull request May 17, 2023

Design notes: app locked #15788

Closed

7 tasks

ajayesivan mentioned this pull request May 17, 2023

fix: permission tag component locked state #15928

Merged

Parveshdhull reviewed May 17, 2023

View reviewed changes

status-github-bot bot moved this from REVIEW to TO REBASE in Pipeline for QA May 17, 2023

ulisesmac reviewed May 17, 2023

View reviewed changes

ajayesivan force-pushed the 15788-design-review-app-locked-ui-updates branch 2 times, most recently from 72150ab to 4fe323f Compare May 17, 2023 23:33

ilmotta approved these changes May 18, 2023

View reviewed changes

ulisesmac approved these changes May 18, 2023

View reviewed changes

ajayesivan force-pushed the 15788-design-review-app-locked-ui-updates branch 2 times, most recently from f59cfe8 to e6022d8 Compare May 18, 2023 21:50

ajayesivan added the request-manual-qa label May 18, 2023

ajayesivan moved this from TO REBASE to E2E Tests in Pipeline for QA May 18, 2023

VolodLytvynenko self-assigned this May 19, 2023

ajayesivan force-pushed the 15788-design-review-app-locked-ui-updates branch 2 times, most recently from 1c83096 to 4706824 Compare May 19, 2023 10:03

VolodLytvynenko moved this from E2E Tests to IN TESTING in Pipeline for QA May 19, 2023

ajayesivan force-pushed the 15788-design-review-app-locked-ui-updates branch 2 times, most recently from ace127f to c993e4b Compare May 19, 2023 11:37

app locked screen design review changes

771d497

ajayesivan force-pushed the 15788-design-review-app-locked-ui-updates branch from c993e4b to 771d497 Compare May 19, 2023 11:51

VolodLytvynenko moved this from IN TESTING to MERGE in Pipeline for QA May 19, 2023

VolodLytvynenko added Tested - OK and removed request-manual-qa labels May 19, 2023

ajayesivan merged commit 7e8f1fa into develop May 19, 2023
6 checks passed

Pipeline for QA automation moved this from MERGE to DONE May 19, 2023

ajayesivan deleted the 15788-design-review-app-locked-ui-updates branch May 19, 2023 12:17

ajayesivan commented May 26, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

App locked screen - design review changes #15927

App locked screen - design review changes #15927

ajayesivan commented May 17, 2023 •

edited

status-im-auto commented May 17, 2023 •

edited

Parveshdhull May 17, 2023

Parveshdhull May 17, 2023

Parveshdhull May 17, 2023

Parveshdhull May 17, 2023

ajayesivan May 17, 2023

ulisesmac May 17, 2023 •

edited

J-Son89 May 17, 2023

ilmotta May 17, 2023

ajayesivan May 17, 2023

ajayesivan May 17, 2023

ulisesmac left a comment

ulisesmac May 17, 2023 •

edited

ajayesivan commented May 17, 2023

ilmotta May 18, 2023

ajayesivan May 18, 2023

status-im-auto commented May 18, 2023

Class TestCommunityMultipleDeviceMerged:

Class TestActivityCenterContactRequestMultipleDevicePR:

Class TestGroupChatMultipleDeviceMergedNewUI:

Class TestActivityMultipleDevicePR:

Class TestOneToOneChatMultipleSharedDevicesNewUi:

Class TestCommunityOneDeviceMerged:

Class TestCommunityMultipleDeviceMerged:

VolodLytvynenko commented May 19, 2023

ajayesivan commented May 19, 2023

VolodLytvynenko commented May 19, 2023

ajayesivan May 26, 2023

App locked screen - design review changes #15927

App locked screen - design review changes #15927

Conversation

ajayesivan commented May 17, 2023 • edited

Fixes

Areas to test

status-im-auto commented May 17, 2023 • edited

Jenkins Builds

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ulisesmac May 17, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ulisesmac left a comment

Choose a reason for hiding this comment

ulisesmac May 17, 2023 • edited

Choose a reason for hiding this comment

ajayesivan commented May 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

status-im-auto commented May 18, 2023

94% of end-end tests have passed

Failed tests (2)

Class TestCommunityMultipleDeviceMerged:

Passed tests (30)

Class TestActivityCenterContactRequestMultipleDevicePR:

Class TestGroupChatMultipleDeviceMergedNewUI:

Class TestActivityMultipleDevicePR:

Class TestOneToOneChatMultipleSharedDevicesNewUi:

Class TestCommunityOneDeviceMerged:

Class TestCommunityMultipleDeviceMerged:

VolodLytvynenko commented May 19, 2023

ajayesivan commented May 19, 2023

VolodLytvynenko commented May 19, 2023

Choose a reason for hiding this comment

ajayesivan commented May 17, 2023 •

edited

status-im-auto commented May 17, 2023 •

edited

ulisesmac May 17, 2023 •

edited

ulisesmac May 17, 2023 •

edited