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: Upgrade TX notifications #241

Open
RedBeardEth opened this issue Jul 11, 2024 · 3 comments
Open

feat: Upgrade TX notifications #241

RedBeardEth opened this issue Jul 11, 2024 · 3 comments
Assignees

Comments

@RedBeardEth
Copy link
Contributor

Describe the feature you'd like to request

image

Notifications in the sidebar are currently generated from:

  1. a zustand local storage store with addTx method called on client tx submissions (e.g claiming Lords)
  2. fetching various data sources (for the Realms nft bridge) in the useTransactions hook and merging with the zustand store above

Describe the solution you'd like to see

The transaction notifications in the account sidebar should:

  • Should store the success state in local storage (at the moment it is fetching tx status from RPC on each open [the ...loading state in the image] if the tx is pending - this should only occur once and update local storage if success).
  • Have an indicator on the root wallet button (e.g green ring or number badge) if unviewed notification (dismissed after viewing)

Additional information

The (currently messy) logic for data fetching will soon be improved (awaiting apibara dna v2 to unify Starknet and L1 data) and more data sources will be added (e.g sales on the nft marketplace) - so the useTransactions hook should be generalised (not too specified to bridge transaction data)

@Dprof-in-tech
Copy link

Hello @RedBeardEth , I'm Isaac a fullstack developer with contributions to multiple projects on onlydust. I Would love to be assigned to this issue.

@Ugo-X
Copy link

Ugo-X commented Jul 20, 2024

Hello Project Lead! @RedBeardEth I'm Ugo, a fullstack(js,React,Node,Next.js,Three.js) developer with a strong track record in OD hack projects. I've been actively involved since Edition 1, contributing to various initiatives.
Having used OnlyDust extensively, I'm confident in my ability to tackle new challenges.
I'm eager to leverage my skills and experience to contribute effectively.
Below is the link to my only dust profile:
https://app.onlydust.com/u/Ugo-X

@4eyes52 4eyes52 self-assigned this Sep 19, 2024
@4eyes52
Copy link
Collaborator

4eyes52 commented Sep 25, 2024

idea for refactor of useTransactions hook @RedBeardEth :

First Part of Refactor:

  • create a new hook called useRealmsBridgeTransactions. Move over most of the logic from useTransactions to this new hook.

  • rename l2TransactionsMap const to be specific to realms bridge transactions

  • return array for pendingwithdrawals and all other realms bridge Transactions from new hook.

  • remove foreach for local storage transactions in useTransactions hook.

  • create a new hook for lordsrewards transactions if one does not already exist to grab the rewards transactions via the existing api query for lordsRewards.

  • new lordsRewards hook returns array of transactions for lords rewards.

  • Import these two hooks into usetransactions, call the hooks there and return the arrays separately (or don't import and keep the hooks separate instead)

  • save these two arrays to local storage with separate localstorage names.

  • have wallet list components display the transactions for lords rewards and realms bridge separately.

    IE transactions list would look like the following:

    Lords Rewards Transactions
    (list of lords rewards transactions)

    Realms Bridge Transactions
    (list of realms bridge transactions)

So the sheet would basically organize the transactions into separate transaction types instead of combining them into one list.

This would provide an initial first step at generalizing the transactions in the wallet sheet and making it easier to differentiate between the types of transactions the sheet currently displays.

Can implement in the working pull request which cleans up storing transactions returned from useTransactions in localstorage: #288

I will update this discussion with second part of refactor idea if this first refactor idea is acceptable @RedBeardEth. Let me know your thoughts!

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

No branches or pull requests

4 participants