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: NanoContractTransactions component [11] #438

Merged
merged 32 commits into from
Jun 12, 2024

Conversation

alexruzenhack
Copy link
Contributor

@alexruzenhack alexruzenhack commented Mar 8, 2024

Acceptance Criteria

  • Should add NanoContractDetailsScreen: it renders details components of a selected Nano Contract
  • Should add NanoContractDetails component: it manages details components and some call to actions
    • Should provide action that navigates a user to the explorer
    • Should provide action to unregister the Nano Contract
    • Should provide action to change Nano Contract's address registered
  • Should add NanoContractDetailsHeader component: it renders some Nano Contract information
  • Should add NanoContractTransactionsListItem: it renders a transaction's summary for each item in a list the Nano Contract's transactions
  • Should add UnregisterNanoContractModal component
  • Should add SelectAddressModal component
  • Should add FeedbackContent component
  • Should remove NoNanoContract component once we already have a generalized component FeedbackContent
  • Should add registeredNanoContractsIter to MemoryStore
  • Should add getRegisteredNanoContracts helper to retrieve the list of registered Nano Contracts
  • Should add action NANOCONTRACT_HISTORY_LOADING to assist the history loading process, and add reducer to handle the event
  • Should add action NANOCONTRACT_INIT and add init effect on sagas/nanoContract
  • Should refactor components to improve its usability and readability
  • Should add feedback content to SelectAddressModal

Nano Contract Details Screen

nc-transactions-recording.mp4

Nano Contract Details loading history

Nano Contract Details after error on transactions history load

Select Address Loading

Select Address Loading Error

Security Checklist

  • Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

@alexruzenhack alexruzenhack self-assigned this Mar 8, 2024
@alexruzenhack alexruzenhack force-pushed the feat/nano-contract-list-component branch 2 times, most recently from a62289f to 243c126 Compare March 31, 2024 23:19
@alexruzenhack alexruzenhack force-pushed the feat/nano-contract-transactions-component branch 2 times, most recently from 1e13c7d to 593332c Compare April 1, 2024 16:02
@alexruzenhack alexruzenhack changed the base branch from feat/nano-contract-list-component to feat/nc-unregister-saga April 5, 2024 17:39
@alexruzenhack alexruzenhack changed the title feat: NanoContractTransactions component feat: NanoContractTransactions component [11] Apr 8, 2024
@alexruzenhack alexruzenhack force-pushed the feat/nc-unregister-saga branch from 674669c to a369c8c Compare May 13, 2024 18:55
@alexruzenhack alexruzenhack force-pushed the feat/nano-contract-transactions-component branch from 593332c to d0fd277 Compare May 14, 2024 15:16
@alexruzenhack alexruzenhack force-pushed the feat/nc-unregister-saga branch 6 times, most recently from e5fdf24 to a56f2fc Compare May 28, 2024 20:18
@alexruzenhack alexruzenhack force-pushed the feat/nano-contract-transactions-component branch 2 times, most recently from a0eb3e7 to ab34b66 Compare May 29, 2024 00:49
@alexruzenhack alexruzenhack force-pushed the feat/nc-unregister-saga branch from a56f2fc to 083f44e Compare May 29, 2024 13:19
@alexruzenhack alexruzenhack force-pushed the feat/nano-contract-transactions-component branch 3 times, most recently from 8787ed7 to 4b08adc Compare May 29, 2024 17:21
locale/da/texts.po Show resolved Hide resolved
locale/pt-br/texts.po Show resolved Hide resolved
locale/pt-br/texts.po Outdated Show resolved Hide resolved
locale/ru-ru/texts.po Show resolved Hide resolved
Base automatically changed from feat/nc-unregister-saga to master May 29, 2024 23:29
@alexruzenhack alexruzenhack force-pushed the feat/nano-contract-transactions-component branch from 4b08adc to 5dd87b1 Compare May 29, 2024 23:30
@alexruzenhack alexruzenhack force-pushed the feat/nano-contract-transactions-component branch from 5dd87b1 to 30f3fe4 Compare May 29, 2024 23:32
src/components/FeedbackContent.js Show resolved Hide resolved
src/utils.js Outdated Show resolved Hide resolved
locale/pt-br/texts.po Outdated Show resolved Hide resolved
locale/pt-br/texts.po Outdated Show resolved Hide resolved
src/reducers/reducer.js Outdated Show resolved Hide resolved
locale/pt-br/texts.po Outdated Show resolved Hide resolved

const log = logger('mixins');

export function* fetchAllWalletAddresses() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this a "mixin"?

Copy link
Contributor Author

@alexruzenhack alexruzenhack Jun 11, 2024

Choose a reason for hiding this comment

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

Because there is a pattern for the sagas, they are being used in cycles and major features that also have cycles. This one doesn't belong to a cycle nor to a major feature. Therefore I created this space for standalone effects. It should aggregate future standalone effects as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand that most of the sagas in src/sagas/wallet.js are related to the wallet load/update "cycle", but not all of them are, like the onResetWallet

IMHO all sagas that are related to the wallet should be in src/sagas/wallet.js, just like all sagas related to "tokens" are in src/sagas/tokens.js

We can discuss having a different structure, like having sagas especifically for those "cycles", but I don't think it should be done in this PR


const log = logger('mixins');

export function* fetchAllWalletAddresses() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand that most of the sagas in src/sagas/wallet.js are related to the wallet load/update "cycle", but not all of them are, like the onResetWallet

IMHO all sagas that are related to the wallet should be in src/sagas/wallet.js, just like all sagas related to "tokens" are in src/sagas/tokens.js

We can discuss having a different structure, like having sagas especifically for those "cycles", but I don't think it should be done in this PR

@alexruzenhack alexruzenhack merged commit efbf1f2 into master Jun 12, 2024
2 checks passed
@alexruzenhack alexruzenhack deleted the feat/nano-contract-transactions-component branch June 12, 2024 21:23
andreabadesso pushed a commit that referenced this pull request Jun 24, 2024
* feat: add nano contract screen
* feat: add nano contract components
* feat: improve NanoContractTransactionsList component
* feat: add init effect on sagas/nanoContract
* refactor: rename NanoContractTransactions* to NanoContractDetails*
* refactor: use consumeAsyncIterator from utils
* refactor: change oneline style to use unicode no-break space
* refactor: extract getAllAddresses as utility fn
* feat: add "offcard" prop to FeedbackContent component
* refactor: move fetch all wallet addresses to a saga effect on sagas/mixins
* refactor: move effects from mixins to wallet
* refactor: remove mixins from sagas/index.js
@tuliomir tuliomir mentioned this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants