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

feat: snaps modal/UX improvements #5169

Merged
merged 41 commits into from
Sep 11, 2023
Merged

feat: snaps modal/UX improvements #5169

merged 41 commits into from
Sep 11, 2023

Conversation

gomesalexandre
Copy link
Contributor

@gomesalexandre gomesalexandre commented Aug 24, 2023 •
edited
Loading

Description

Add a snaps CTA modal on MM connect, which will be displayed on next connect unless "Don't ask again" is checked, or the snap is actually added.

Pull Request Type

  • 🐛 Bug fix (Non-breaking Change: Fixes an issue)
  • 🛠️ Chore (Non-breaking Change: Doc updates, pkg upgrades, typos, etc..)
  • 💅 New Feature (Breaking/Non-breaking Change)

Issue (if applicable)

N/A

Risk

Low, ensure feature flagging of new branches is looking good here

Testing

Snappy fingers flag on

  • Same as the PRs below this on how to run snaps
  • Ensure you see the snaps CTA modal after connecting MM Flask
  • Ensure clicking "Add Snap" while not having the snap installed installs it
  • Ensure having the snap already added and clearing your persisted store than hard refreshing doesn't trigger the modal again (since the snap is already installed and should be detected as such)
  • Ensure connecting with regular MM doesn't trigger the modal
  • Ensure not toggling "Never ask again" and closing without installing triggers the modal on refresh

Snappy fingers flag off

  • This should never pop up with the flag off

Engineering

  • ☝🏽

Operations

  • Do a general regression of everything that implies a wallet with the flag off, sends/receives, trades (can do a cheap one), Defi opportunities being shown etc

Screenshots (if applicable)

image

@gomesalexandre gomesalexandre mentioned this pull request Aug 24, 2023
Merged
@gomesalexandre Graphite App
Copy link
Contributor Author

gomesalexandre commented Aug 24, 2023 •
edited by woodenfurniture
Loading

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@gomesalexandre gomesalexandre mentioned this pull request Aug 24, 2023
Closed
3 tasks
gomesalexandre
gomesalexandre commented Aug 24, 2023
View reviewed changes
src/components/Modals/Snaps/Snaps.tsx Outdated
const allNativeAssets = useMemo(() => {
return Object.values(KnownChainIds)
.map(knownChainId => {
const assetId = getChainAdapterManager().get(knownChainId)?.getFeeAssetId()!
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We may or may not want to filter out EVM chains depending on whether or not they work as fall through with MM snaps in the end - if they don't, we have bigger problems

@gomesalexandre gomesalexandre marked this pull request as ready for review August 24, 2023 20:36
@gomesalexandre gomesalexandre requested a review from a team as a code owner August 24, 2023 20:36
@gomesalexandre
Copy link
Contributor Author

Back to draft as this https://github.com/shapeshift/web/pull/5169/files#diff-642b012440aeafd61512683e8b249ef64289ae5a0e38ff6a5c0143c4c3b23484 looks like it breaks things in swapper, including with the flag off

@gomesalexandre gomesalexandre marked this pull request as draft August 24, 2023 21:14
@gomesalexandre gomesalexandre changed the base branch from feat_snaps_demo_accounts to feat_snaps_btn September 6, 2023 16:46
@gomesalexandre gomesalexandre force-pushed the feat_snaps_modal branch 2 times, most recently from 624dd32 to 7c1a1c6 Compare September 7, 2023 13:13
@gomesalexandre gomesalexandre changed the title feat: snaps modal feat: snaps modal/UX improvements Sep 11, 2023
@gomesalexandre gomesalexandre marked this pull request as ready for review September 11, 2023 20:29
@gomesalexandre gomesalexandre merged commit 91b50fe into feat_snaps_btn Sep 11, 2023
@gomesalexandre gomesalexandre deleted the feat_snaps_modal branch September 11, 2023 20:29
@woodenfurniture woodenfurniture mentioned this pull request Sep 11, 2023
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
beard-oil
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gomesalexandre @reallybeard