fixes Not visible status bar on light theme #21105

Parveshdhull · 2024-08-21T13:11:04Z

Summary

The issue occurred because we updated the theme value after setting the root (not before), which caused the root to be initialized with the old theme value.

PR only addresses bug fixing because its urgent. Other improvements related to naming and doc-string are done in #21107

status: ready

status-im-auto · 2024-08-21T13:15:39Z

Jenkins Builds

Click to see older builds (8)

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
❌	`d66c80d`	#1	2024-08-21 13:15:39	~4 min	`tests`	📄`log`

✔️	`a22d15f`	#2	2024-08-21 13:21:56	~4 min	`tests`	📄`log`

✔️	`a308b95`	#4	2024-08-21 13:30:40	~4 min	`tests`	📄`log`
✔️	`a308b95`	#4	2024-08-21 13:32:57	~7 min	`android-e2e`	🤖`apk` 📲
✔️	`a308b95`	#4	2024-08-21 13:33:23	~7 min	`android`	🤖`apk` 📲

✔️	`852829e`	#5	2024-08-21 13:45:00	~4 min	`tests`	📄`log`
✔️	`852829e`	#5	2024-08-21 13:47:07	~6 min	`android`	🤖`apk` 📲
✔️	`852829e`	#5	2024-08-21 13:48:43	~8 min	`android-e2e`	🤖`apk` 📲

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`a5c63e6`	#6	2024-08-21 14:01:28	~4 min	`tests`	📄`log`
✔️	`a5c63e6`	#6	2024-08-21 14:03:42	~7 min	`android-e2e`	🤖`apk` 📲
✔️	`a5c63e6`	#6	2024-08-21 14:04:01	~7 min	`android`	🤖`apk` 📲
✔️	`a5c63e6`	#6	2024-08-21 14:13:23	~16 min	`ios`	📱`ipa` 📲

✔️	`351d68f`	#7	2024-08-22 11:34:46	~4 min	`tests`	📄`log`
✔️	`351d68f`	#7	2024-08-22 11:37:31	~7 min	`android-e2e`	🤖`apk` 📲
✔️	`351d68f`	#7	2024-08-22 11:37:52	~7 min	`android`	🤖`apk` 📲
✔️	`351d68f`	#7	2024-08-22 11:47:10	~16 min	`ios`	📱`ipa` 📲

ilmotta

LGTM, thanks @Parveshdhull

ilmotta · 2024-08-21T13:41:19Z

src/status_im/navigation/events.cljs

+(rf/defn update-theme-and-init-root
+  {:events [:update-theme-and-init-root]}
+  [_ root-id]
+  {:fx [[:dispatch [:theme/switch {:view-id root-id}]]


Looks like a proper solution to use ordering from :fx 👍🏼, but haven't tested to be sure the fix is good.

ilmotta · 2024-08-21T13:45:07Z

src/status_im/contexts/onboarding/welcome/view.cljs

@@ -45,7 +45,7 @@
          :colors [colors/neutral-100-opa-0 colors/neutral-100-opa-80]}])
      [quo/button
       {:on-press            (fn []
-                               (rf/dispatch [:init-root :shell-stack])
+                               (rf/dispatch [:update-theme-and-init-root :shell-stack])


It's not the first, nor will be the last time that I think we should have some convention for names of events that are sort of private and should not be used directly. Clearly from now on, dispatching :init-root will lead to potential bugs, so something about the name of :init-root should indicate that. A comment or docstring is not as potent.

And idea for us to think about :)

Thank you @ilmotta for reminding. I wrote the docstring, but forget to add before pushing. (Added now)

Created an issue, we will explore it. Thank you
#21108

status-im-auto · 2024-08-22T10:54:36Z

86% of end-end tests have passed

Total executed tests: 7
Failed tests: 1
Expected to fail tests: 0
Passed tests: 6

IDs of failed tests: 702745

Failed tests (1)

Click to expand

Rerun failed tests

Class TestOneToOneChatMultipleSharedDevicesNewUi:

1. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745

Device 1: Tap on found: Button
Device 1: Find `Button` by `accessibility id`: `Crop`

critical/chats/test_1_1_public_chats.py:278: in test_1_1_chat_non_latin_messages_stack_update_profile_photo
    self.profile_1.edit_profile_picture(image_index=2)
../views/profile_view.py:459: in edit_profile_picture
    self.crop_photo_button.click()
../views/base_element.py:90: in click
    element = self.find_element()
../views/base_element.py:84: in find_element
    raise exception
../views/base_element.py:77: in find_element
    return self.driver.find_element(self.by, self.locator)
../../../../status-app-prs@tmp/venv/lib/python3.10/site-packages/appium/webdriver/webdriver.py:409: in find_element
    return self.execute(RemoteCommand.FIND_ELEMENT, {'using': by, 'value': value})['value']
../../../../status-app-prs@tmp/venv/lib/python3.10/site-packages/selenium/webdriver/remote/webdriver.py:345: in execute
    self.error_handler.check_response(response)
../../../../status-app-prs@tmp/venv/lib/python3.10/site-packages/appium/webdriver/errorhandler.py:122: in check_response
    raise exception_class(msg=message, stacktrace=format_stacktrace(stacktrace))
 An unknown server-side error occurred while processing the command. Original error: 'POST /element' cannot be proxied to UiAutomator2 server because the instrumentation process is not running (probably crashed). Check the server log and/or the logcat output for more details
E   Stacktrace:
E   UnknownError: An unknown server-side error occurred while processing the command. Original error: 'POST /element' cannot be proxied to UiAutomator2 server because the instrumentation process is not running (probably crashed). Check the server log and/or the logcat output for more details
E       at UIA2Proxy.command (/mnt/sauce/appium/appium-v2.0.0/packages/base-driver/lib/jsonwp-proxy/proxy.js:355:13)
E       at processTicksAndRejections (node:internal/process/task_queues:95:5)
E       at AndroidUiautomator2Driver.helpers.doFindElementOrEls (/mnt/sauce/appium/appium-v2.0.0/appium_home/linux/node_modules/appium-uiautomator2-driver/lib/commands/find.js:30:10)
E       at doFind (/mnt/sauce/appium/appium-v2.0.0/appium_home/linux/node_modules/appium-uiautomator2-driver/node_modules/appium-android-driver/lib/commands/find.js:36:17)
E       at wrappedCondFn (/mnt/sauce/appium/appium-v2.0.0/packages/base-driver/lib/basedriver/commands/timeout.ts:137:14)
E       at spin (/mnt/sauce/appium/appium-v2.0.0/node_modules/asyncbox/lib/asyncbox.js:221:20)
E       at waitForCondition (/mnt/sauce/appium/appium-v2.0.0/node_modules/asyncbox/lib/asyncbox.js:238:10)
E       at AndroidUiautomator2Driver.implicitWaitForCondition (/mnt/sauce/appium/appium-v2.0.0/packages/base-driver/lib/basedriver/commands/timeout.ts:139:12)
E       at AndroidUiautomator2Driver.findElOrEls (/mnt/sauce/appium/appium-v2.0.0/appium_home/linux/node_modules/appium-uiautomator2-driver/node_modules/appium-android-driver/lib/commands/find.js:60:5)
E       at AndroidUiautomator2Driver.findElOrElsWithProcessing (/mnt/sauce/appium/appium-v2.0.0/packages/base-driver/lib/basedriver/commands/find.ts:60:12)
E       at AndroidUiautomator2Driver.findElement (/mnt/sauce/appium/appium-v2.0.0/packages/base-driver/lib/basedriver/commands/find.ts:75:12)

Device sessions

Device 1:

Steps, video, logs

Device 2:

Steps, video, logs

Passed tests (6)

Click to expand

Class TestCommunityOneDeviceMerged:

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

Class TestWalletMultipleDevice:

1. test_wallet_send_asset_from_drawer, id: 727230

2. test_wallet_send_eth, id: 727229

Class TestWalletOneDevice:

1. test_wallet_add_remove_regular_account, id: 727231
Device sessions Device 1: Steps, video, logs

Class TestCommunityMultipleDeviceMerged:

1. test_community_message_edit, id: 702843
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs

status-im-auto · 2024-08-22T11:13:46Z

100% of end-end tests have passed

Total executed tests: 1
Failed tests: 0
Expected to fail tests: 0
Passed tests: 1

Passed tests (1)

Click to expand

Class TestOneToOneChatMultipleSharedDevicesNewUi:

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

mariia-skrypnyk · 2024-08-22T11:17:51Z

@Parveshdhull thanks for fix.

Ready to be merged and cherry-picked.

The issue occurred because we updated the theme value after setting the root (not before), which caused the root to be initialized with the old theme value.

mariia-skrypnyk · 2024-10-25T10:43:05Z

Hi @Parveshdhull @ilmotta !

I observe the same behaviour on release iOS build.

When user change theme from dark to light the status bar becomes invisible.
It can be fixed by relogin and we discussed it previously.
But user can not know about it and this issue is low prio but very noticeable.
Can it be investigated once more?

Parveshdhull · 2024-10-25T11:00:29Z

Hi @mariia-skrypnyk,

Thank you very much for bringing up this issue.

Unfortunately, this is a limitation or bug within the library, and we are currently unable to fix it. You can find more details here: #15596 and #19089

RNN Bug - We are also changing the status-bar-theme at runtime, but for some reason theme used at the time of root initialization has high priority and gets restored on navigation (going back from chat, etc.)

I’m not certain, but hopefully, if we migrate to the React Navigation library, this issue can be resolved. (Currently out of scope?)

cc @flexsurfer

ilmotta · 2024-10-25T13:00:06Z

I'm not certain, but hopefully, if we migrate to the React Navigation library, this issue can be resolved. (Currently out of scope?)

Hey @Parveshdhull, to give some visibility here, the scope of 2.32 is still being defined. Just today Volo and I aligned that we need to proactively start to assess from scratch our push notification solution so that by 2.33 we can do something about it because it's very strategic to the success of Status, and it's not currently working in our favor. Another example, next week there will be a call to define the Keycard scope for 2.32. So as you can see, there are other things in the scope and the migration to React Navigation may need to wait a bit longer. Most likely will be in the scope 2.33.

Parveshdhull force-pushed the fix/status-bar-bug branch 4 times, most recently from a308b95 to 852829e Compare August 21, 2024 13:39

Parveshdhull mentioned this pull request Aug 21, 2024

Improve app theme naming and doc strings #21107

Merged

Parveshdhull self-assigned this Aug 21, 2024

Parveshdhull added the request-manual-qa label Aug 21, 2024

Parveshdhull requested review from flexsurfer and ilmotta August 21, 2024 13:45

ilmotta approved these changes Aug 21, 2024

View reviewed changes

Parveshdhull mentioned this pull request Aug 21, 2024

Add documentation/naming-convention for private events that should not be called directly #21108

Open

mariia-skrypnyk self-assigned this Aug 22, 2024

Parveshdhull added 2 commits August 22, 2024 08:29

fixes Not visible status bar on light theme

454041e

add warning

351d68f

ilmotta force-pushed the fix/status-bar-bug branch from a5c63e6 to 351d68f Compare August 22, 2024 11:30

ilmotta merged commit 449b280 into develop Aug 22, 2024
5 checks passed

ilmotta deleted the fix/status-bar-bug branch August 22, 2024 11:54

cammellos pushed a commit that referenced this pull request Aug 22, 2024

fix: Not visible status bar on light theme (#21105)

93f7826

The issue occurred because we updated the theme value after setting the root (not before), which caused the root to be initialized with the old theme value.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixes Not visible status bar on light theme #21105

fixes Not visible status bar on light theme #21105

Parveshdhull commented Aug 21, 2024 •

edited

Loading

status-im-auto commented Aug 21, 2024 •

edited

Loading

ilmotta left a comment

ilmotta Aug 21, 2024

ilmotta Aug 21, 2024

Parveshdhull Aug 21, 2024

Parveshdhull Aug 21, 2024

status-im-auto commented Aug 22, 2024

Class TestOneToOneChatMultipleSharedDevicesNewUi:

Class TestCommunityOneDeviceMerged:

Class TestWalletMultipleDevice:

Class TestWalletOneDevice:

Class TestCommunityMultipleDeviceMerged:

status-im-auto commented Aug 22, 2024

Class TestOneToOneChatMultipleSharedDevicesNewUi:

mariia-skrypnyk commented Aug 22, 2024

mariia-skrypnyk commented Oct 25, 2024

Parveshdhull commented Oct 25, 2024

ilmotta commented Oct 25, 2024

fixes Not visible status bar on light theme #21105

fixes Not visible status bar on light theme #21105

Conversation

Parveshdhull commented Aug 21, 2024 • edited Loading

Summary

status-im-auto commented Aug 21, 2024 • edited Loading

Jenkins Builds

ilmotta left a comment

Choose a reason for hiding this comment

ilmotta Aug 21, 2024

Choose a reason for hiding this comment

ilmotta Aug 21, 2024

Choose a reason for hiding this comment

Parveshdhull Aug 21, 2024

Choose a reason for hiding this comment

Parveshdhull Aug 21, 2024

Choose a reason for hiding this comment

status-im-auto commented Aug 22, 2024

86% of end-end tests have passed

Failed tests (1)

Class TestOneToOneChatMultipleSharedDevicesNewUi:

Passed tests (6)

Class TestCommunityOneDeviceMerged:

Class TestWalletMultipleDevice:

Class TestWalletOneDevice:

Class TestCommunityMultipleDeviceMerged:

status-im-auto commented Aug 22, 2024

100% of end-end tests have passed

Passed tests (1)

Class TestOneToOneChatMultipleSharedDevicesNewUi:

mariia-skrypnyk commented Aug 22, 2024

mariia-skrypnyk commented Oct 25, 2024

Parveshdhull commented Oct 25, 2024

ilmotta commented Oct 25, 2024

Parveshdhull commented Aug 21, 2024 •

edited

Loading

status-im-auto commented Aug 21, 2024 •

edited

Loading