Skip to content

Fix wrong placeholder alignment and broken autofill #31044

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

Merged
merged 3 commits into from
Nov 8, 2023
Merged

Fix wrong placeholder alignment and broken autofill #31044

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
8b15c46
fix autofill on signinpage
getusha Nov 8, 2023
de36894
fix wrong placeholder alignment
getusha Nov 8, 2023
703230e
fix lint
getusha Nov 8, 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
3 changes: 2 additions & 1 deletion src/components/Composer/index.android.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ function Composer({shouldClear, onClear, isDisabled, maxLines, forwardedRef, isC
// @Szymon20000 is working on fixing this (android-only) issue in the in the upstream PR (https://github.com/facebook/react-native/pulls?q=is%3Apr+is%3Aopen+maxNumberOfLines)
// TODO: remove this comment once upstream PR is merged and available in a future release
maxNumberOfLines={maxNumberOfLines}
style={[composerStyles, styles.verticalAlignMiddle]}
textAlignVertical="center"
style={[composerStyles]}
/* eslint-disable-next-line react/jsx-props-no-spreading */
{...props}
readOnly={isDisabled}
Expand Down
3 changes: 2 additions & 1 deletion src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,8 @@ function ComposerWithSuggestions({
placeholderTextColor={themeColors.placeholderText}
onChangeText={(commentValue) => updateComment(commentValue, true)}
onKeyPress={triggerHotkeyActions}
style={[styles.textInputCompose, isComposerFullSize ? styles.textInputFullCompose : styles.flex4, styles.verticalAlignTop]}
textAlignVertical="top"
Copy link
Contributor

@aimane-chnaif aimane-chnaif Nov 8, 2023

Choose a reason for hiding this comment

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

textAlignVertical prop is deprecated. There are still 2 usages in latest main. Let's replace them as well

Copy link
Contributor Author

@getusha getusha Nov 8, 2023

Choose a reason for hiding this comment

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

@aimane-chnaif replacing them caused this issue, they're not accurately replacement, i will check if there is any other option.

Copy link
Contributor Author

@getusha getusha Nov 8, 2023

Choose a reason for hiding this comment

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

I replaced textAlignVertical="top" with styles.verticalAlignTop before, i just reverted that here.

Copy link
Contributor

@aimane-chnaif aimane-chnaif Nov 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

If it's not exact replacement, there should be issue created in upstream. I doubt the maintainers wrote document even without testing.

Copy link
Contributor Author

@getusha getusha Nov 8, 2023

Choose a reason for hiding this comment

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

@aimane-chnaif it's not working for our case, it should be working correctly after the replacement but it's not. the only other option is to remove styles.verticalAlignTop and do not add the prop.

style={[styles.textInputCompose, isComposerFullSize ? styles.textInputFullCompose : styles.flex4]}
maxLines={maxComposerLines}
onFocus={onFocus}
onBlur={onBlur}
Expand Down
1 change: 0 additions & 1 deletion src/pages/signin/LoginForm/BaseLoginForm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,6 @@ function LoginForm(props) {
ref={input}
label={translate('loginForm.phoneOrEmail')}
accessibilityLabel={translate('loginForm.phoneOrEmail')}
role={CONST.ACCESSIBILITY_ROLE.TEXT}
Copy link
Contributor

@situchan situchan Nov 8, 2023

Choose a reason for hiding this comment

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

@bernhardoj as you reproduced autofill issue, can you double confirm that removing this line fixes it?

Copy link
Contributor Author

@getusha getusha Nov 8, 2023

Choose a reason for hiding this comment

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

@situchan I am able to reproduce, and yes it will fix it.

Before

Screen.Recording.2023-11-08.at.11.11.11.AM.mov

After

Screen.Recording.2023-11-08.at.11.13.06.AM.mov

Copy link
Contributor

@situchan situchan Nov 8, 2023

Choose a reason for hiding this comment

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

Seems like you're using autofill extension.
I am using default chrome browser autofill.

Copy link
Contributor Author

@getusha getusha Nov 8, 2023

Choose a reason for hiding this comment

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

@situchan it is the default chrome browser autofill, the manage option will redirect you to chrome://settings/autofill

Copy link
Contributor

@bernhardoj bernhardoj Nov 8, 2023

Choose a reason for hiding this comment

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

Yes, I can confirm it fixes the issue.

value={login}
returnKeyType="go"
autoCompleteType="username"
Expand Down