Wallet - Implement Network Configuration #17840 #17862
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (77)
|
|
The demo looks strange, icons are blinking, seems like they re-rendered but they shouldn't be |
I noticed this problem, but I don't know how to fix it yet |
Rende11
force-pushed
the
wallet/network-configuration
branch
from
008c206 to
940d5a5
Compare
|
Looks like "blinking" - component issue (or I use it in wrong way) Simulator.Screen.Recording.-.iPhone.13.-.2023-11-11.at.18.08.59.mp4 |
Rende11
force-pushed
the
wallet/network-configuration
branch
2 times, most recently
from
0a8a4ab to
f7680bf
Compare
Rende11
commented
Nov 13, 2023
J-Son89
requested review from
OmarBasem,
J-Son89,
vkjr,
tumanov-alex,
mmilad75,
briansztamfater and
ulisesmac
I think this function is not enough to know what's the problem. I'll check the code. @Rende11 Please provide the namespace of the component |
ulisesmac
reviewed
Nov 14, 2023
There was a problem hiding this comment.
Hey @Rende11
I left some comments.
Also I think the name for the subscription and related values is network-preferences and I've seen the name network-preference (not plural).
I wonder if this is important or not, in general I think is better to preserve the concept names
src/status_im2/contexts/wallet/common/sheets/network_preferences/view.cljs
Outdated
Show resolved
Hide resolved
src/status_im2/contexts/wallet/common/sheets/network_preferences/view.cljs
Outdated
Show resolved
Hide resolved
src/status_im2/contexts/wallet/common/sheets/network_preferences/view.cljs
Outdated
Show resolved
Hide resolved
vkjr
reviewed
Nov 15, 2023
src/status_im2/contexts/wallet/common/sheets/network_preferences/view.cljs
Outdated
Show resolved
Hide resolved
@ulisesmac |
Rende11
force-pushed
the
wallet/network-configuration
branch
3 times, most recently
from
ae2ca9f to
3c5992b
Compare
Please let's sovle it in a different issue since it's an existing problem in the component, not related to this PR 👍 |
Rende11
force-pushed
the
wallet/network-configuration
branch
2 times, most recently
from
f042206 to
e182225
Compare
|
@VolodLytvynenko |
84% of end-end tests have passedFailed tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Expected to fail tests (7)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Passed tests (41)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestDeepLinksOneDevice:
Class TestActivityMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
|
flexsurfer
approved these changes
Dec 8, 2023
src/status_im2/contexts/wallet/common/sheets/network_preferences/view.cljs
Show resolved
Hide resolved
|
hi @Rende11 Could you please rebase and resolve the existing conflicts in the current PR? Thanx |
|
@Rende11 The integration tests are failed https://ci.status.im/job/status-mobile/job/prs/job/tests/job/PR-17862/22/consoleText could you check, please? |
I don't have access to this link, but I will check tests locally |
Rende11
force-pushed
the
wallet/network-configuration
branch
3 times, most recently
from
8a598de to
7ccdaa8
Compare
Updated and fixed |
73% of end-end tests have passedFailed tests (6)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Expected to fail tests (7)Click to expandClass TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Passed tests (36)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestDeepLinksOneDevice:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
|
hey @Rende11 thank you for PR. No issues from my side. PR is ready to be merged |
VolodLytvynenko
added
Tested - OK
test cases required
and removed
Tested - Issues
labels
Check chains Save network-preferences Improvements Fix linter issues Clean up Updates Update sub test Update tests Updates Store as set Fix minor issues Update naming Revive `get-account-by-address` Fix alignment Fix merge Fix btn label Fix desc Split state Fix test Format tests
Rende11
force-pushed
the
wallet/network-configuration
branch
from
7ccdaa8 to
6c90f43
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #17840
Summary
Account network configuration
This pr adds the network configuration page. This can accessed by going to any wallet account page (new UI)
and opening the edit account option.
Once there you can edit network configuration. (These changes will eventually be reflected in many parts of the application. However in this pr these changes are persisted in the db and can be visualised on the account page in the form of the full address name which will optionally include "eth", "arb1" (and/or) "opt".
For this pr we can test that this is working as expected and that these changes are persisted on log in/log out. Additionally the network configuration page should load correctly with the previously selected network configuration. By default they will all be selected.
See the video below
https://github.com/status-im/status-mobile/assets/10757633/318f2249-0a6f-404f-a5a8-c2ac64f17f69