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.

Sign up for GitHub

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

fix: nonce increment/decrement functionality using arrow buttons #26569

Merged
merged 12 commits into from
Sep 3, 2024
Merged

fix: nonce increment/decrement functionality using arrow buttons #26569

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
8299329
fix: increment custom nonce using arrow up and down
vinistevam Aug 21, 2024
965fbce
fix unit test
vinistevam Aug 21, 2024
21a4e91
Merge branch 'develop' into fix/custom-nonce-increment-btns
vinistevam Aug 21, 2024
a93466b
Merge branch 'develop' into fix/custom-nonce-increment-btns
vinistevam Aug 21, 2024
05b1f86
Merge branch 'develop' into fix/custom-nonce-increment-btns
vinistevam Aug 21, 2024
b99a238
Merge branch 'develop' into fix/custom-nonce-increment-btns
sleepytanya Aug 21, 2024
adcdc61
Merge branch 'develop' into fix/custom-nonce-increment-btns
vinistevam Aug 22, 2024
0b112a5
Merge branch 'develop' into fix/custom-nonce-increment-btns
vinistevam Aug 22, 2024
2698877
applied suggestions
vinistevam Aug 22, 2024
2e30805
fix: only update custom nonce when nonce field renders
vinistevam Aug 22, 2024
8cbf8be
Merge branch 'develop' into fix/custom-nonce-increment-btns
vinistevam Aug 27, 2024
a9b1358
Merge branch 'develop' into fix/custom-nonce-increment-btns
vinistevam Sep 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion ...onfirmations/confirm-transaction-base/__snapshots__/confirm-transaction-base.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ exports[`Confirm Transaction Base should match snapshot 1`] = `
<div
class="confirm-page-container-summary__nonce"
>
#undefined
#70
</div>
</div>
<div
Expand Down Expand Up @@ -459,6 +459,33 @@ exports[`Confirm Transaction Base should match snapshot 1`] = `
</div>
</div>
</div>
<div>
<div
class="confirm-detail-row"
>
<div
class="confirm-detail-row__label"
>
Custom nonce
</div>
<div
class="custom-nonce-input"
>
<div
class="mm-box mm-text-field mm-text-field--size-md mm-text-field--truncate mm-box--padding-right-0 mm-box--padding-left-0 mm-box--display-inline-flex mm-box--align-items-center mm-box--background-color-background-default mm-box--rounded-sm mm-box--border-width-1 box--border-style-solid"
min="0"
>
<input
autocomplete="off"
class="mm-box mm-text mm-input mm-input--disable-state-styles mm-text-field__input mm-text--body-md mm-box--margin-0 mm-box--padding-0 mm-box--padding-right-4 mm-box--padding-left-4 mm-box--color-text-default mm-box--background-color-transparent mm-box--border-style-none"
focused="false"
type="number"
value="70"
/>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
Expand Down
38 changes: 24 additions & 14 deletions ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.component.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
import UserPreferencedCurrencyDisplay from '../../../components/app/user-preferenced-currency-display';

import { PRIMARY, SECONDARY } from '../../../helpers/constants/common';
import TextField from '../../../components/ui/text-field';
import { MetaMetricsEventCategory } from '../../../../shared/constants/metametrics';
import { getMethodName } from '../../../helpers/utils/metrics';
import {
Expand All @@ -42,7 +41,7 @@ import {
import { TransactionModalContextProvider } from '../../../contexts/transaction-modal';
import TransactionDetail from '../components/transaction-detail/transaction-detail.component';
import TransactionDetailItem from '../components/transaction-detail-item/transaction-detail-item.component';
import { Text } from '../../../components/component-library';
import { Text, TextField } from '../../../components/component-library';
import LoadingHeartBeat from '../../../components/ui/loading-heartbeat';
import LedgerInstructionField from '../components/ledger-instruction-field';
import {
Expand Down Expand Up @@ -431,6 +430,19 @@ export default class ConfirmTransactionBase extends Component {
);
};

const handleNonceChange = ({ target: { value } }) => {
Copy link
Member

Choose a reason for hiding this comment

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

Minor, do we want a useCallback just in case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

100% we need to use the usaCallback, but in this case, it is an old class component.

const inputValue = Number(value);

if (inputValue < 0 || isNaN(inputValue)) {
updateCustomNonce('');
Copy link
Member

Choose a reason for hiding this comment

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

Would it be cleaner to set this to undefined to be explicit as we already have the fallback on 533?

return;
}

updateCustomNonce(String(inputValue));
Copy link
Member

Choose a reason for hiding this comment

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

Assuming the inputValue is a decimal and not hexadecimal, why are we storing it as a string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, no need to store it as a string, removed.


getNextNonce();
};

const renderTotalMaxAmount = ({
useMaxFee,
isBoldTextAndNotOverridden = false,
Expand Down Expand Up @@ -496,6 +508,13 @@ export default class ConfirmTransactionBase extends Component {
: primaryTotalTextOverride;
};

const nextNonceValue =
typeof nextNonce === 'number' ? nextNonce.toString() : null;

if (!customNonceValue && nextNonceValue) {
updateCustomNonce(nextNonceValue);
}

const nonceField =
useNonceField && !isSigningOrSubmitting ? (
<div>
Expand All @@ -507,20 +526,11 @@ export default class ConfirmTransactionBase extends Component {
<TextField
type="number"
min={0}
placeholder={
typeof nextNonce === 'number' ? nextNonce.toString() : null
}
onChange={({ target: { value } }) => {
if (!value.length || Number(value) < 0) {
updateCustomNonce('');
} else {
updateCustomNonce(String(Math.floor(value)));
}
getNextNonce();
}}
placeholder={nextNonceValue}
onChange={handleNonceChange}
fullWidth
margin="dense"
value={customNonceValue || ''}
value={customNonceValue ?? ''}
/>
</div>
</div>
Expand Down
2 changes: 2 additions & 0 deletions ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,8 @@ const baseStore = {
tokenList: {},
ensResolutionsByAddress: {},
snaps: {},
useNonceField: true,
customNonceValue: '70',
},
confirmTransaction: {
txData: {
Expand Down