Remove unused transaction base props#10295
Conversation
These props were never given. They have been removed to simplify the component.
106e514 to
eef7dd8
Compare
Builds ready [eef7dd8]
Page Load Metrics (674 ± 45 ms)
|
| @@ -19,7 +19,6 @@ export default class ConfirmPageContainerContent extends Component { | |||
| assetImage: PropTypes.string, | |||
There was a problem hiding this comment.
Going to add review and comments file by file, to ensure my own thinking is good.
| nonce={nonce} | ||
| assetImage={assetImage} | ||
| /> | ||
| {this.renderContent()} |
There was a problem hiding this comment.
This file looks good. ConfirmPageContainerContent is only used in ConfirmPageContainer, which is only used in ConfirmTransactionBase, the instances of which are never passed a summary component.
| summaryComponent={summaryComponent} | ||
| detailsComponent={detailsComponent} | ||
| dataComponent={dataComponent} | ||
| errorMessage={errorMessage} |
There was a problem hiding this comment.
This file looks good as per https://github.com/MetaMask/metamask-extension/pull/10295/files#r566206507
| warning: PropTypes.string, | ||
| advancedInlineGasShown: PropTypes.bool, | ||
| insufficientBalance: PropTypes.bool, | ||
| hideFiatConversion: PropTypes.bool, |
There was a problem hiding this comment.
changes to here in this file look good. I verified that none of these properties are passed to any of the instances of ConfrimTransactionBaseComponent
| </div> | ||
| ) : null} | ||
| </div> | ||
| ) |
There was a problem hiding this comment.
above big group of changes is just indentation change
| action={functionType} | ||
| title={title} | ||
| titleComponent={this.renderTitleComponent()} | ||
| subtitle={subtitle} |
There was a problem hiding this comment.
I think subtitle can be removed from ConfirmPageContainer and down through the component tree, as was done with summaryComponent
| disabled={!valid || submitting} | ||
| onEdit={() => this.handleEdit()} | ||
| onCancelAll={() => this.handleCancelAll()} | ||
| onCancel={() => this.handleCancel()} |
danjm
left a comment
There was a problem hiding this comment.
Left one suggested change: https://github.com/MetaMask/metamask-extension/pull/10295/files#r566221054
looks good otherwise
Builds ready [3b7176a]
Page Load Metrics (826 ± 66 ms)
|
These props were never given. They have been removed to simplify the component.