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

Fix: Entering the incorrect magic code spins the 'sign-in' button continuously offline #18960

Merged
merged 10 commits into from
May 19, 2023
40 changes: 29 additions & 11 deletions src/components/MagicCodeInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,15 @@ import CONST from '../CONST';
import Text from './Text';
import TextInput from './TextInput';
import FormHelpMessage from './FormHelpMessage';
import {withNetwork} from './OnyxProvider';
import networkPropTypes from './networkPropTypes';
import useOnNetworkReconnect from './hooks/useOnNetworkReconnect';
import * as Browser from '../libs/Browser';

const propTypes = {
/** Information about the network */
network: networkPropTypes.isRequired,

/** Name attribute for the input */
name: PropTypes.string,

Expand Down Expand Up @@ -103,16 +109,22 @@ function MagicCodeInput(props) {
},
}));

useEffect(() => {
// Blurs the input and removes focus from the last input and, if it should submit
// on complete, it will call the onFulfill callback.
const validateAndSubmit = () => {
const numbers = decomposeString(props.value);
if (!props.shouldSubmitOnComplete || _.filter(numbers, (n) => ValidationUtils.isNumeric(n)).length !== CONST.MAGIC_CODE_LENGTH) {
if (!props.shouldSubmitOnComplete || _.filter(numbers, (n) => ValidationUtils.isNumeric(n)).length !== CONST.MAGIC_CODE_LENGTH || props.network.isOffline) {
return;
}
// Blurs the input and removes focus from the last input and, if it should submit
// on complete, it will call the onFulfill callback.
inputRefs.current[editIndex].blur();
setFocusedIndex(undefined);
props.onFulfill(props.value);
};

useOnNetworkReconnect(validateAndSubmit);

useEffect(() => {
validateAndSubmit();
Copy link
Member

@rushatgabhane rushatgabhane Jun 26, 2023

Choose a reason for hiding this comment

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

Hi, just dropping a note that this PR might have missed a case, causing the bug - "Incorrect magic code" error if click "Go back" in 2FA input page

Explaination of the problem - #19712 (comment)


// We have not added the editIndex as the dependency because we don't want to run this logic after focusing on an input to edit it after the user has completed the code.
// eslint-disable-next-line react-hooks/exhaustive-deps
Expand Down Expand Up @@ -255,6 +267,10 @@ function MagicCodeInput(props) {
setEditIndex(newFocusedIndex);
inputRefs.current[newFocusedIndex].focus();
} else if (keyValue === 'Enter') {
// we shouldn also prevent user from submit here when it's offline.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// we shouldn also prevent user from submit here when it's offline.
//We should prevent users from submitting when it's offline.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, just updated, Thank you @puneetlath

if (props.network.isOffline) {
return;
}
setInput('');
props.onFulfill(props.value);
}
Expand Down Expand Up @@ -321,10 +337,12 @@ function MagicCodeInput(props) {
MagicCodeInput.propTypes = propTypes;
MagicCodeInput.defaultProps = defaultProps;

export default forwardRef((props, ref) => (
<MagicCodeInput
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
innerRef={ref}
/>
));
export default withNetwork()(
forwardRef((props, ref) => (
<MagicCodeInput
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
innerRef={ref}
/>
)),
);
7 changes: 3 additions & 4 deletions src/components/Pressable/PressableWithFeedback.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,9 @@ const PressableWithFeedback = forwardRef((props, ref) => {
setDisabled(props.disabled);
return;
}
onPress
.finally(() => {
setDisabled(props.disabled);
});
onPress.finally(() => {
setDisabled(props.disabled);
});
});
}}
>
Expand Down