-
Notifications
You must be signed in to change notification settings - Fork 993
Chore: Add :screen qualifier to all screens #22413
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
Conversation
Jenkins BuildsClick to see older builds (12)
|
eadca54
to
94adeb2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed unused file
@@ -99,7 +99,7 @@ | |||
(update :chats-home-list set/difference removed-chats)) | |||
:fx [(when (not-empty removed-chats) | |||
[:effects/push-notifications-clear-message-notifications removed-chats]) | |||
(when (and (= view-id :chat) (removed-chats current-chat-id)) | |||
(when (and (= view-id :screen/chat) (removed-chats current-chat-id)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most problematic screen name was :chat
. Many strings starting with :chat
(let [community-id (or id (rf/sub [:get-screen-params :community-overview])) | ||
(let [community-id (or id (quo.context/use-screen-params)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leftover from #22293 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @ulisesmac
@@ -2,30 +2,31 @@ | |||
|
|||
(def ^:const floating-shell-button-height 44) | |||
|
|||
(def ^:const default-selected-stack :wallet-stack) | |||
(def ^:const default-selected-stack :screen/wallet-stack) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only side effect of this migration is that we store the last selected stack ID in async storage, which will be something like :communities-stack. Since this no longer exists, the selected stack ID will fall back to :screen/wallet-stack for first run after update
94adeb2
to
2d12df1
Compare
Hey @mobile-qa, the PR doesn't need manual testing (just a rename), but could you please run the full E2E suite? It affects a lot of files (82). |
87% of end-end tests have passed
Failed tests (2)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Expected to fail tests (1)Click to expandClass TestWalletOneDevice:
Passed tests (20)Click to expandClass TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Class TestWalletOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletCollectibles:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletOneDeviceTwo:
Class TestProfileMultipleDevices:
Class TestWalletMultipleDevice:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very much needed refactoring! Thank you @Parveshdhull !
@Parveshdhull This one should still be addressed: #22388 also a consequence of the new hook |
fixes #22358
Testing Note:
PR don't need manual testing, but please run full e2e test suite. (Affects many file, but only rename)
status: ready