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

Remove notifications for completed transactions #860

Merged
merged 3 commits into from
Nov 15, 2024

Conversation

kkosiorowska
Copy link
Contributor

@kkosiorowska kkosiorowska commented Nov 14, 2024

This PR removes notifications for completed transactions. What has been done:

  • Removed ActivitiesList and ActivitiesListItem components.
  • Removed unneeded variant "process" for Alert.
  • Removed unneeded types and utils.
  • Removed unneeded code for latestActivities

UI

Before
Screenshot 2024-11-14 at 10 52 35

After
Screenshot 2024-11-14 at 10 53 16

We no longer need this logic because we have removed the notifications.
@kkosiorowska kkosiorowska self-assigned this Nov 14, 2024
Copy link

netlify bot commented Nov 14, 2024

Deploy Preview for acre-dapp-testnet ready!

Name Link
🔨 Latest commit dd8cb6b
🔍 Latest deploy log https://app.netlify.com/sites/acre-dapp-testnet/deploys/6736faa3642ec70008b4418a
😎 Deploy Preview https://deploy-preview-860--acre-dapp-testnet.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Nov 14, 2024

Deploy Preview for acre-dapp ready!

Name Link
🔨 Latest commit dd8cb6b
🔍 Latest deploy log https://app.netlify.com/sites/acre-dapp/deploys/6736faa3d63573000814c3e4
😎 Deploy Preview https://deploy-preview-860--acre-dapp.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@kkosiorowska kkosiorowska marked this pull request as ready for review November 14, 2024 09:55
Copy link
Contributor

@r-czajkowski r-czajkowski left a comment

Choose a reason for hiding this comment

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

The code looks good to me, but I'm leaving the final approval + merge to @michalinacienciala .

Copy link
Contributor

@michalinacienciala michalinacienciala left a comment

Choose a reason for hiding this comment

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

Tested for the deposit in LL. For withdrawal in the standalone dapp. During testing of withdrawal there was some Gelato error thrown, but it looks inconsequential (funds got returned, withdrawal visible in dapp) and could not be reproduced. I think we can merge.

@michalinacienciala michalinacienciala merged commit 92f66fd into main Nov 15, 2024
28 checks passed
@michalinacienciala michalinacienciala deleted the delete-transaction-notifications branch November 15, 2024 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants