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

Jump to bottom

Fix - Login field is focused when sign out in small screen devices #24026

Merged
merged 3 commits into from
Aug 5, 2023
Merged

Fix - Login field is focused when sign out in small screen devices #24026

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d9b97ce
Only focus if form re visible again
hoangzinh Aug 1, 2023
73c1e7e
set value default value to prevent it's alway undefined initially
hoangzinh Aug 1, 2023
283cb9d
Update comment
hoangzinh Aug 4, 2023
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
2 changes: 1 addition & 1 deletion src/hooks/usePrevious.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {useEffect, useRef} from 'react';
* @returns {*}
*/
export default function usePrevious(value) {
const ref = useRef();
const ref = useRef(value);
Copy link
Contributor Author

@hoangzinh hoangzinh Aug 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

During testing this PR, I found that the prevProp and nextProp always different in beginning. I.e, in this case, !prevIsVisible && props.isVisible is always true in beginning, because prevIsVisible is initialized with undefined, then if props.isVisible has value, !prevIsVisible && props.isVisible is always true in beginning. To solve this issue, I think we have 2 options:

  • Option 1: Always check an addition condition that prevIsVisible is not undefined as well before checking if prevIsVisible is different with props.isVisible.
    Pros: We don't touch existing code => It's might not cause any breaking changes.
    Cons: It might be our tech debt/code smell later.

  • Option 2: Fix this LOC, I think we should pass input value as a initialize value of ref.
    Pros: If we fix here, I think it will work properly/correctly in the future for checking previousProps and nextProps
    Cons: It might cause breaking changes. We need to test all current places using it. I have tested a few of them:

    • FAB, Sign in form, WS member
Screen.Recording.2023-08-03.at.07.04.50.mp4
Screen.Recording.2023-08-03.at.09.45.46.mp4

Copy link
Member

Choose a reason for hiding this comment

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

I am fine with either.

useEffect(() => {
ref.current = value;
}, [value]);
Expand Down
8 changes: 6 additions & 2 deletions src/pages/signin/LoginForm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import isInputAutoFilled from '../../libs/isInputAutoFilled';
import * as PolicyUtils from '../../libs/PolicyUtils';
import Log from '../../libs/Log';
import withNavigationFocus, {withNavigationFocusPropTypes} from '../../components/withNavigationFocus';
import usePrevious from '../../hooks/usePrevious';

const propTypes = {
/** Should we dismiss the keyboard when transitioning away from the page? */
Expand Down Expand Up @@ -81,6 +82,7 @@ function LoginForm(props) {
const input = useRef();
const [login, setLogin] = useState('');
const [formError, setFormError] = useState(false);
const prevIsVisible = usePrevious(props.isVisible);

const {translate} = props;

Expand Down Expand Up @@ -166,11 +168,13 @@ function LoginForm(props) {
if (props.blurOnSubmit) {
input.current.blur();
}
if (!input.current || !props.isVisible) {

// Only focus if the form is re-visible again to prevent the keyboard is shown in touchscreen devices after sign out
hoangzinh marked this conversation as resolved.
Show resolved Hide resolved
if (!input.current || prevIsVisible || !props.isVisible) {
return;
}
input.current.focus();
}, [props.blurOnSubmit, props.isVisible, input]);
}, [props.blurOnSubmit, props.isVisible, prevIsVisible]);

const formErrorText = useMemo(() => (formError ? translate(formError) : ''), [formError, translate]);
const serverErrorText = useMemo(() => ErrorUtils.getLatestErrorMessage(props.account), [props.account]);
Expand Down
Loading