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

generalize error/success message state #799

Closed
Jascha-Sundaresan opened this issue Oct 17, 2017 · 5 comments
Closed

generalize error/success message state #799

Jascha-Sundaresan opened this issue Oct 17, 2017 · 5 comments
Assignees

Comments

@Jascha-Sundaresan
Copy link
Contributor

Jascha-Sundaresan commented Oct 17, 2017

currently, the global app state is gigantic and a large portion of it is in the control object:

control: {
    // NextAddress
    getNextAddressRequestAttempt: false,
    getNextAddressResponse: null,
    getNextAddressError: null,
    // RenameAccount
    renameAccountRequestAttempt: false,
    renameAccountResponse: null,
    renameAccountSuccess: null,
    renameAccountError: null,
    // Rescan
    rescanRequestAttempt: false,
    rescaneRequest: null,
    rescanResponse: null,
    rescanError: null,
    // NextAccount
    getNextAccountRequestAttempt: false,
    getNextAccountResponse: null,
    getNextAccountSuccess: null,
    getNextAccountError: null,
    // ImportPrivateKey
    importPrivateKeyRequestAttempt: false,
    importPrivateKeyResponse: null,
    importPrivateKeyError: null,
    // ImportScript
    importScriptRequestAttempt: false,
    importScriptResponse: null,
    importScriptError: null,
    importScriptSuccess: "",
    // ChangePassphrase
    changePassphraseRequestAttempt: false,
    changePassphraseResponse: null,
    changePassphraseError: null,
    changePassphraseSuccess: "",
    // ChangePassphrase
    loadActiveDataFiltersRequestAttempt: false,
    loadActiveDataFiltersResponse: null,
    loadActiveDataFiltersError: null,
    // FundTransaction
    fundTransactionRequestAttempt: false,
    fundTransactionResponse: null,
    fundTransactionError: null,
    // SignTransaction
    signTransactionRequestAttempt: false,
    signTransactionRespsonse: null,
    signTransactionError: null,
    // PublishTransaction
    publishTransactionRequestAttempt: false,
    publishTransactionResponse: null,
    publishTransactionError: null,
    // PurchaseTicket
    purchaseTicketsRequestAttempt: false,
    purchaseTicketsResponse: null,
    purchaseTicketsSuccess: "",
    purchaseTicketsError: null,
    // RevokeTickets
    revokeTicketsRequestAttempt: false,
    revokeTicketsResponse: null,
    revokeTicketsSuccess: "",
    revokeTicketsError: null,

    // TicketBuyerService
    ticketBuyerService: null,
    // TicketBuyerConfig
    balanceToMaintain: cfg.get("balancetomaintain"),
    maxFee: cfg.get("maxfee"),
    maxPriceAbsolute: cfg.get("maxpriceabsolute"),
    maxPriceRelative: cfg.get("maxpricerelative"),
    maxPerBlock: cfg.get("maxperblock"),
    getTicketBuyerConfigRequestAttempt: false,
    getTicketBuyerConfigResponse: null,
    getTicketBuyerConfigSuccess: null,
    getTicketBuyerConfigError: null,
    // StartAutoBuyer
    startAutoBuyerRequestAttempt: false,
    startAutoBuyerResponse: null,
    startAutoBuyerSuccess: null,
    startAutoBuyerError: null,
    // StopAutoBuyer
    stopAutoBuyerRequestAttempt: false,
    stopAutoBuyerResponse: null,
    stopAutoBuyerSuccess: null,
    stopAutoBuyerError: null,
    // ConstructTransaction
    constructTxRequestAttempt: false,
    constructTxResponse: null,
    constructTxError: null,
  },

I propose we refactor this to simply:

request: {
 loading: false,
 response: null,
 message: "",
 error: false,
 type: "" // "construcTx", "stopAutoBuyer", etc...

This will dramatically reduce the overhead in the props we're passing around to our page headers.

@matheusd
Copy link
Member

I had actually partly started work on that on #774.

One of the problems we had was that the header, sidebar and snackbar components were replicated on every page instead of being set on a single unified layout.

We had a couple of commits/PRs refactoring those out of individual views (#774 is the latest). Moving the snackbar out of the header, it will be possible to collapse all those error states into a single error notification queue.

@alexlyp
Copy link
Member

alexlyp commented Oct 18, 2017

I like the direction of this proposal, though I think it should be coordinated with @go1dfish's current work refactoring the actions. So I'd start with doing a PoC on a set of actions that has already been merged, then if we like it we can apply the same changes to the rest.

@Jascha-Sundaresan
Copy link
Contributor Author

ack

@matheusd
Copy link
Member

IMO, the way forward for this is to go to the new snackbar reducer (https://github.com/decred/decrediton/blob/master/app/reducers/snackbar.js) and replace every *error/*success state that is only a message (not, say a disabled control) with a snackbar message. We can then style all messages appropriately.

@Jascha-Sundaresan
Copy link
Contributor Author

this is exactly what I had in mind @matheusd 👍

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