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

Adds background to label #5959

Merged
merged 7 commits into from
Oct 20, 2021
+26 −10
Merged

Adds background to label #5959

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
562ad4e
Adds background to label
AlfredoAlc Oct 20, 2021
cc08d6c
Alingned closing bracket
AlfredoAlc Oct 20, 2021
93a79ba
Changes label background height to correct height
AlfredoAlc Oct 20, 2021
8635982
Adds comment explaining the use of label background
AlfredoAlc Oct 20, 2021
336c913
Adds overflow prop to textinput container
AlfredoAlc Oct 20, 2021
e6e6f89
Modifies styling syntax
AlfredoAlc Oct 20, 2021
d045e55
Merge branch 'Expensify:main' into alfredo-fix-textinput-area
AlfredoAlc Oct 20, 2021
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
25 changes: 15 additions & 10 deletions src/components/ExpensiTextInput/BaseExpensiTextInput.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,16 +171,21 @@ class BaseExpensiTextInput extends Component {
]}
>
{hasLabel ? (
<ExpensiTextInputLabel
label={label}
labelTranslateX={
ignoreLabelTranslateX
? new Animated.Value(0)
: this.state.labelTranslateX
}
labelTranslateY={this.state.labelTranslateY}
labelScale={this.state.labelScale}
/>
<>
{/* Adding this background to the label only for multiline text input,
to prevent text overlaping with label when scrolling */}
{multiline && <View style={styles.expensiTextInputLabelBackground} />}
Copy link
Contributor

Choose a reason for hiding this comment

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

Am I correct in thinking that the only reason we don't add this view for single-line inputs is because of the effect it causes when an input is auto-filled? Did you see or investigate this comment?

Copy link
Contributor Author

@AlfredoAlc AlfredoAlc Oct 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, it is only set to multiline to avoid seeing the white background when it isn't multiline and it has autofill. And it look all the container blue(or whatever the color set depending on the browser).

I did investigate on it but there are some points to consider when trying to change the background set by the browser.

See #5704 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, well I don't think we need to worry too much about the styles applied by the browser's auto-fill, so this is probably okay for now.

<ExpensiTextInputLabel
label={label}
labelTranslateX={
ignoreLabelTranslateX
? new Animated.Value(0)
: this.state.labelTranslateX
}
labelTranslateY={this.state.labelTranslateY}
labelScale={this.state.labelScale}
/>
</>
) : null}
<TextInput
ref={(ref) => {
Expand Down
11 changes: 11 additions & 0 deletions src/styles/styles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,7 @@ const styles = {
justifyContent: 'center',
height: '100%',
backgroundColor: themeColors.componentBG,
overflow: 'hidden',
},
expensiTextInputLabel: {
position: 'absolute',
Expand All @@ -531,6 +532,15 @@ const styles = {
fontFamily: fontFamily.GTA,
width: '100%',
},
expensiTextInputLabelBackground: {
position: 'absolute',
top: 0,
width: '100%',
height: 25,
backgroundColor: themeColors.componentBG,
borderTopRightRadius: variables.componentBorderRadiusNormal,
borderTopLeftRadius: variables.componentBorderRadiusNormal,
},
expensiTextInputLabelDesktop: {
transformOrigin: 'left center',
},
Expand All @@ -550,6 +560,7 @@ const styles = {
paddingBottom: 8,
paddingHorizontal: 11.5,
borderRadius: variables.componentBorderRadiusNormal,
zIndex: -1,
Copy link
Member

Choose a reason for hiding this comment

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

Please test it and it should not have any regressions.

},
expensiTextInputDesktop: addOutlineWidth({}, 0),
expensiTextInputAndroid: left => ({
Expand Down