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.

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

feat(suite-native): new device switcher #12300

Merged
merged 1 commit into from
May 15, 2024

Conversation

vytick
Copy link
Contributor

@vytick vytick commented May 7, 2024

Update wallet switcher visual to show view only devices correctly

  • Generic device icon changed to model-specific
  • Show current device connection state
  • Show/hide device selector on button tap
  • Show all devices including connection state (view only)
  • Make device selector slide out/in animated
  • Show all wallets for current device
  • Hide Open passphrase button behind feature flag and make it disabled during discovery
  • New connected state for View only settings harmonised with new design
  • New visual for current device/wallet when closed manager

These two tasks will be dealt with in following PR so this one is not even bigger

  • Show wallet fiat amount
  • Scrolling wallets to fit size

Related Issue

Part of #11892

Screenshots:

Simulator.Screen.Recording.-.iPhone.15.-.2024-05-07.at.22.15.43.mp4

@vytick vytick added the mobile Suite Lite issues and PRs label May 7, 2024
Copy link

socket-security bot commented May 7, 2024

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

Ignoring: npm/[email protected]

View full report↗︎

Next steps

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

@vytick
Copy link
Contributor Author

vytick commented May 7, 2024

@SocketSecurity ignore-all

@vytick vytick force-pushed the feat/native/device-switcher-wallets branch 4 times, most recently from c2f2504 to 2ea26ef Compare May 7, 2024 20:28
@vytick vytick marked this pull request as ready for review May 7, 2024 20:43
@vytick vytick requested a review from a team as a code owner May 7, 2024 20:43
Copy link
Contributor

@juriczech juriczech left a comment

Choose a reason for hiding this comment

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

Mostly just nitpicks, but what seems strange is the need to type cast devices and optionality of some of the component props.

juriczech

This comment was marked as duplicate.

@vytick vytick force-pushed the feat/native/device-switcher-wallets branch from fb7e2fd to 326abb2 Compare May 9, 2024 20:30
Copy link
Contributor

@PeKne PeKne left a comment

Choose a reason for hiding this comment

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

.

@vytick vytick force-pushed the feat/native/device-switcher-wallets branch from 5825a46 to 043485d Compare May 13, 2024 10:14
Copy link
Contributor

@juriczech juriczech left a comment

Choose a reason for hiding this comment

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

THANKS FOR RESOLVING ALL OF THE COMMENTS 🎉

Tested on emulator on android and works well with remember and passphrase as well.

Really nice animation, well done.

Next time, it would be better for review and development to split into smaller PR's but maybe it was unexpected to be this big 😮

@matejkriz
Copy link
Member

matejkriz commented May 15, 2024

RESOLVED
I just spotted missing padding below Connect another device button in case of multiple devices

image

Copy link
Contributor

@PeKne PeKne left a comment

Choose a reason for hiding this comment

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

👏 Thanks for addressing my comments.

@vytick
Copy link
Contributor Author

vytick commented May 15, 2024

@matejkriz Good catch with layout glitch! fixed in a5016a6

@vytick
Copy link
Contributor Author

vytick commented May 15, 2024

/rebase

Copy link

@trezor-ci trezor-ci force-pushed the feat/native/device-switcher-wallets branch from eb117cf to 3cdd926 Compare May 15, 2024 14:05
@vytick vytick merged commit fcac90d into develop May 15, 2024
74 checks passed
@vytick vytick deleted the feat/native/device-switcher-wallets branch May 15, 2024 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mobile Suite Lite issues and PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants