Skip to content

Commit

Permalink
Fix autolock field to accept decimals in Firefox
Browse files Browse the repository at this point in the history
The autolock field on the Settings screen — the field that allows users
to set the duration that MetaMask will wait for until automatically
locking — does not always accept decimal numbers. This breaks the e2e
test for this feature as it attempts to set this field to "0.1".

More specifically, the React component responsible for this field passes
whatever the user inputs through the `Number` function immediately and
then uses this to repopulate the input. Therefore, if the user enters
"3" followed by a ".", `Number("3.")` will be called. This evaluates to
the number 3, and "3" becomes the new value of the field. As a result,
the "." can never be typed.

Curiously, this behavior only happens in Firefox; Chrome seems to
keep the "." in the input field when it's typed. This happens because
`onChange` event doesn't seem to get fired until a number is typed
*after* the ".". This may be due to underlying differences in the DOM
between Chrome and Firefox.

Regardless, always passing the input through `Number` creates other odd
behavior, such as the fact that the input can never be cleared (because
`Number("")` evaluates to 0).

This commit solves these problems by saving the "raw" version of the
user's input as well as the normalized version. The raw version is
always used to populate the input, whereas the normalized version is
saved in state.
  • Loading branch information
mcmire committed Jun 18, 2023
1 parent 8edcc6a commit d5f9bdf
Show file tree
Hide file tree
Showing 24 changed files with 40 additions and 75 deletions.
3 changes: 0 additions & 3 deletions app/_locales/de/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions app/_locales/el/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions app/_locales/en/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions app/_locales/es/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions app/_locales/es_419/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions app/_locales/fr/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions app/_locales/hi/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions app/_locales/id/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions app/_locales/it/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions app/_locales/ja/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions app/_locales/ko/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions app/_locales/ph/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions app/_locales/pt/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions app/_locales/pt_BR/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions app/_locales/ru/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions app/_locales/tl/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions app/_locales/tr/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions app/_locales/vi/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions app/_locales/zh_CN/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions app/_locales/zh_TW/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion test/e2e/tests/auto-lock.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('Auto-Lock Timer', function () {
await autoLockTimerInput.fill(10081);
await driver.waitForSelector({
css: '#autoTimeout-helper-text',
text: 'Lock time is too great',
text: 'Lock time must be a number between 0 and 10080',
});
await autoLockTimerInput.fill(sixSecsInMins);
await driver.assertElementNotPresent('#autoTimeout-helper-text');
Expand Down
2 changes: 2 additions & 0 deletions ui/components/ui/text-field/text-field.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,8 @@ const TextField = ({
inputProps.InputProps.inputProps['data-testid'] = dataTestId;
}

// inputProps.InputProps.inputProps.step = '0.1';

return (
<MaterialTextField
error={Boolean(error)}
Expand Down
46 changes: 33 additions & 13 deletions ui/pages/settings/advanced-tab/advanced-tab.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export default class AdvancedTab extends PureComponent {

state = {
autoLockTimeLimit: this.props.autoLockTimeLimit,
autoLockTimeLimitBeforeNormalization: this.props.autoLockTimeLimit,
lockTimeError: '',
showLedgerTransportWarning: false,
showResultMessage: false,
Expand Down Expand Up @@ -379,11 +380,10 @@ export default class AdvancedTab extends PureComponent {
<div className="settings-page__content-item">
<div className="settings-page__content-item-col">
<TextField
type="number"
id="autoTimeout"
data-testid="auto-lockout-time"
placeholder="5"
value={this.state.autoLockTimeLimit}
value={this.state.autoLockTimeLimitBeforeNormalization}
onChange={(e) => this.handleLockChange(e.target.value)}
error={lockTimeError}
fullWidth
Expand Down Expand Up @@ -601,21 +601,41 @@ export default class AdvancedTab extends PureComponent {
);
}

handleLockChange(time) {
handleLockChange(autoLockTimeLimitBeforeNormalization) {
const { t } = this.context;
const autoLockTimeLimit = Math.max(Number(time), 0);

this.setState(() => {
let lockTimeError = '';
if (autoLockTimeLimitBeforeNormalization === '') {
this.setState({
autoLockTimeLimitBeforeNormalization,
autoLockTimeLimit: '5',
lockTimeError: '',
});
return;
}

if (autoLockTimeLimit > 10080) {
lockTimeError = t('lockTimeTooGreat');
}
const autoLockTimeLimitAfterNormalization = Number(
autoLockTimeLimitBeforeNormalization,
);

return {
autoLockTimeLimit,
lockTimeError,
};
if (
Number.isNaN(autoLockTimeLimitAfterNormalization) ||
autoLockTimeLimitAfterNormalization < 0 ||
autoLockTimeLimitAfterNormalization > 10080
) {
this.setState({
autoLockTimeLimitBeforeNormalization,
autoLockTimeLimit: null,
lockTimeError: t('lockTimeInvalid'),
});
return;
}

const autoLockTimeLimit = autoLockTimeLimitAfterNormalization;

this.setState({
autoLockTimeLimitBeforeNormalization,
autoLockTimeLimit,
lockTimeError: '',
});
}

Expand Down
4 changes: 2 additions & 2 deletions ui/pages/settings/advanced-tab/advanced-tab.component.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ describe('AdvancedTab Component', () => {
const autoLockoutTime = queryByTestId('auto-lockout-time');
const autoLockoutButton = queryByTestId('auto-lockout-button');

fireEvent.change(autoLockoutTime, { target: { value: 1440 } });
fireEvent.change(autoLockoutTime, { target: { value: '1440' } });

expect(autoLockoutTime).toHaveValue(1440);
expect(autoLockoutTime).toHaveValue('1440');

fireEvent.click(autoLockoutButton);

Expand Down

0 comments on commit d5f9bdf

Please sign in to comment.