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.

Sign up for GitHub

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

Jump to bottom

[MC 0.5][FEATURE] - Wallet actions on Tab bar #5992

Merged
merged 31 commits into from
Apr 4, 2023
Merged

[MC 0.5][FEATURE] - Wallet actions on Tab bar #5992

merged 31 commits into from
Apr 4, 2023
+1,532 −473

Conversation

tommasini
Copy link
Contributor

@tommasini tommasini commented Mar 18, 2023 •
edited

Description
This PR aims to implement a new tab bar option with the wallet actions

Screenshots/Recordings

Issue

Progresses #

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

@tommasini tommasini added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Mar 18, 2023
@tommasini tommasini requested a review from a team as a code owner March 18, 2023 05:37
@github-actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@Cal-L
Copy link
Contributor

Cal-L commented Mar 21, 2023

Have you checked if the new and legacy toasts are still aligned properly relative to the tab bar? What about the onboarding wizard, is that also aligned correctly?

@tommasini
Copy link
Contributor Author

Onboarding wizard, it's okay: https://recordit.co/IPlRPzzVJL
Legacy and new toasts: https://recordit.co/13HP2FMsoQ

There is a little bit more of margin-bottom on the new toasts, do you know if this was intended @Cal-L ?

@Cal-L
Copy link
Contributor

Cal-L commented Mar 22, 2023

Onboarding wizard, it's okay: https://recordit.co/IPlRPzzVJL

Legacy and new toasts: https://recordit.co/13HP2FMsoQ

There is a little bit more of margin-bottom on the new toasts, do you know if this was intended @Cal-L ?

For the new toast, it's normal since it respects both the bottom notch and the curvature of the bottom edges. We should probably push the legacy toast up a bit. If it's too complicated, I'd leave it alone since we'll refactor that anyways.

As for the onboarding wizard, can we push down the step that points at the browser?

For the wallet actions sheet, the padding on the top between the notch and the first option looks like it could use some padding.

@tommasini
Copy link
Contributor Author

Thanks @Cal-L for the review and eagle eyes!

Onboarding wizard browser step fixed:

Onboarding wizard edit account name step fixed:

Padding on send while it just has two options:

The legacy toast padding seems to not be that easy, let's save it for future refactoring to the new Toast 🙏

Cal-L
Cal-L reviewed Mar 29, 2023
View reviewed changes
Copy link
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

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

Left some comments

app/components/UI/WalletAction/WalletAction.tsx Outdated Show resolved Hide resolved
app/component-library/components/Sheet/SheetBottom/SheetBottom.tsx Outdated Show resolved Hide resolved
app/component-library/components/Sheet/SheetBottom/SheetBottom.tsx Outdated Show resolved Hide resolved
app/component-library/components/Sheet/SheetBottom/SheetBottom.types.ts Outdated Show resolved Hide resolved
app/components/UI/WalletAction/WalletAction.styles.ts Outdated Show resolved Hide resolved
app/components/UI/WalletAction/WalletAction.tsx Outdated Show resolved Hide resolved
app/components/Views/WalletActions/WalletActions.styles.ts Outdated Show resolved Hide resolved
Cal-L
Cal-L approved these changes Mar 31, 2023
View reviewed changes
Copy link
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

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

LGTM

@tommasini tommasini added needs-qa Any New Features that needs a full manual QA prior to being added to a release. and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Mar 31, 2023
@tommasini tommasini mentioned this pull request Mar 31, 2023
Merged
3 tasks
@SamuelSalas SamuelSalas added QA in Progress QA has started on the feature. and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Mar 31, 2023
@SamuelSalas
E2e appium/650 tab bar actions (#6086)
b44cbd5 
* solve

* solve

* Fix test scripts

* Update send erc token detox test

* Add pause to send eth flow test script

* Increment pause to send eth flow test script

* Remove pause

* Snapshot updated

* Network feature test data updated
@SamuelSalas SamuelSalas added QA Passed A successful QA run through has been done and removed QA in Progress QA has started on the feature. labels Apr 3, 2023
@tommasini tommasini merged commit df2a70b into main Apr 4, 2023
@tommasini tommasini deleted the feature/650-tab-bar-actions branch April 4, 2023 10:40
@github-actions github-actions bot locked and limited conversation to collaborators Apr 4, 2023
@gauthierpetetin gauthierpetetin changed the title [FEATURE][MC] - Wallet actions on Tab bar [MC 0.5][FEATURE] - Wallet actions on Tab bar Apr 10, 2023
@gauthierpetetin gauthierpetetin added the team-mobile-ux DEPRECATED: please use "team-wallet-ux" label instead label Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sethkfman sethkfman sethkfman left review comments

@Cal-L Cal-L Cal-L approved these changes

@SamuelSalas SamuelSalas SamuelSalas approved these changes

Assignees
No one assigned
Labels
QA Passed A successful QA run through has been done release-6.4.0 PR for release 6.4.0 team-mobile-ux DEPRECATED: please use "team-wallet-ux" label instead
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tommasini @Cal-L @sethkfman @SamuelSalas @gauthierpetetin