Skip to content

Override ownProps with state props in SignatureRequest#6911

Merged
Gudahtt merged 1 commit intoMetaMask:developfrom
Gudahtt:prioritize-state-props-in-signature-request
Jul 26, 2019
Merged

Override ownProps with state props in SignatureRequest#6911
Gudahtt merged 1 commit intoMetaMask:developfrom
Gudahtt:prioritize-state-props-in-signature-request

Conversation

@Gudahtt
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt commented Jul 25, 2019

The accounts prop of SignatureRequest was throwing a PropType warning because accounts was an object instead of an array. It looks like when the mergeProps function was added in #6340, the ownProps were accidentally set to override the state props.

The now ignored props have been removed from the parent ConfirmTxScreen component as well. conversionRate was identical to the one retrieved in SignatureRequest, and selectedAddress differed only in the fallback behaviour when state.metamask.selectedAddress does not exist; it will now default to the first account instead (as it did prior to #6340).

@Gudahtt Gudahtt requested review from danjm and whymarrh as code owners July 25, 2019 02:49
The `accounts` prop of `SignatureRequest` was throwing a PropType
warning because `accounts` was an object instead of an array. It looks
like when the `mergeProps` function was added in MetaMask#6340, the ownProps
were accidentally set to override the state props.

The now ignored props have been removed from the parent `ConfirmTxScreen`
component as well. `conversionRate` was identical to the one retrieved
in `SignatureRequest`, and `selectedAddress` differed only in the
fallback behaviour when `state.metamask.selectedAddress` does not exist;
it will now default to the first account instead (as was the original
behavior, prior to MetaMask#6340).
@Gudahtt Gudahtt force-pushed the prioritize-state-props-in-signature-request branch from 6c7a0b8 to 685560a Compare July 25, 2019 02:50
@Gudahtt Gudahtt merged commit 1f45798 into MetaMask:develop Jul 26, 2019
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.

2 participants