Skip to content

Text & Dropdown Input Styles #12447

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
Luke9389 merged 55 commits into from
Jan 25, 2023
Merged

Text & Dropdown Input Styles #12447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
5dc12f2
started changing text input styles
Nov 3, 2022
6ee9c62
text inputs are done (except for colors)
Nov 3, 2022
48f1916
picker stuff
Nov 3, 2022
e695c5d
medium pickers
Nov 3, 2022
d1b5304
increase border width
Nov 3, 2022
355ab21
Merge branch 'main' of github.com:Expensify/App into luke-update-text…
Dec 6, 2022
544750a
bad merge
Dec 6, 2022
999525c
fixed focus problem
Dec 6, 2022
54b5430
on to the picker
Dec 6, 2022
c4a3f7a
got picker lookin good on web
Dec 6, 2022
7c8a096
background colors need to be transparent
Dec 6, 2022
a37c048
padding left for andoid
Dec 6, 2022
6727994
picker padding left
Dec 6, 2022
a548e9a
padding left for android
Dec 6, 2022
81afaef
on focus lineHeight issue
Dec 6, 2022
bfd3865
took out unused styles
Dec 7, 2022
fb74a32
use paddingHorizontal instead
Dec 7, 2022
b5a6bd3
Merge branch 'main' of github.com:Expensify/App into luke-update-text…
Dec 12, 2022
52bb267
took out show password button border radius
Dec 12, 2022
2e885c8
generalized styles.picker
Dec 13, 2022
a3415b2
use new styles.picker function
Dec 13, 2022
bf2d4d3
removed picker style files
Dec 13, 2022
266dfe0
removed import
Dec 13, 2022
51b1a75
moved picker into parent folder
Dec 13, 2022
b57c985
put textAlign: left back
Dec 13, 2022
243e830
took out right padding for dropdowns
Dec 13, 2022
81530b9
change input height variable
Dec 13, 2022
5bb34e3
dangling comma
Dec 13, 2022
e7a1f56
remove compose box border radius
Dec 13, 2022
94925f7
slight adjustment to dropdown height
Dec 14, 2022
96c8fca
dang ole dangling comma got me again
Dec 14, 2022
fb0f758
Merge branch 'main' of github.com:Expensify/App into luke-update-text…
Jan 3, 2023
1d3ac19
paths were broken from bad merge
Jan 3, 2023
98b31fc
fixed front page locale picker height
Jan 3, 2023
ff0590d
used margin for edit message issue
Jan 3, 2023
307d4ec
Merge branch 'main' of github.com:Expensify/App into luke-update-text…
Jan 4, 2023
d34c4cc
Merge branch 'main' of github.com:Expensify/App into luke-update-text…
Jan 6, 2023
2ac0245
horizontal overlap issue
Jan 6, 2023
7935ce7
padding left to padding horizontal
Jan 6, 2023
7ed732b
containerStyles fix
Jan 6, 2023
af7f53a
right padding for small picker
Jan 6, 2023
725ae01
delete picker styles
Jan 6, 2023
a05191d
padding right on picker
Jan 6, 2023
3bc3aa9
fixed border bottom on IOU
Jan 9, 2023
6e87f30
Merge branch 'main' of github.com:Expensify/App into luke-update-text…
Jan 9, 2023
933a08c
typo in merge resolution
Jan 9, 2023
dbd2f42
Merge branch 'main' of github.com:Expensify/App into luke-update-text…
Jan 11, 2023
ed95537
line height change
Jan 11, 2023
866f2c0
outline width
Jan 12, 2023
e5e0e1a
Merge branch 'main' of github.com:Expensify/App into luke-update-text…
Jan 17, 2023
266902b
set horizontal padding differently
Jan 17, 2023
d9acaf0
don't set padding zero
Jan 17, 2023
4471cb7
add padding left
Jan 20, 2023
e93f98f
input height small should be 28
Jan 24, 2023
508c9fd
fix small picker
Jan 24, 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: 2 additions & 0 deletions src/components/LocalePicker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import CONST from '../CONST';
import Permissions from '../libs/Permissions';
import * as Localize from '../libs/Localize';
import Picker from './Picker';
import styles from '../styles/styles';

const propTypes = {
/** Indicates which locale the user currently has selected */
Expand Down Expand Up @@ -59,6 +60,7 @@ const LocalePicker = (props) => {
items={_.values(localesToLanguages)}
size={props.size}
value={props.preferredLocale}
containerStyles={[props.size === 'small' && [styles.pickerContainerSmall]]}
/>
);
};
Expand Down
17 changes: 8 additions & 9 deletions src/components/Picker/index.js → src/components/Picker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@ import React, {PureComponent} from 'react';
import {View} from 'react-native';
import PropTypes from 'prop-types';
import RNPickerSelect from 'react-native-picker-select';
import Icon from '../Icon';
import * as Expensicons from '../Icon/Expensicons';
import FormHelpMessage from '../FormHelpMessage';
import Text from '../Text';
import styles from '../../styles/styles';
import themeColors from '../../styles/themes/default';
import pickerStyles from './pickerStyles';
import {ScrollContext} from '../ScrollViewWithContext';
import Icon from './Icon';
import * as Expensicons from './Icon/Expensicons';
import FormHelpMessage from './FormHelpMessage';
import Text from './Text';
import styles from '../styles/styles';
import themeColors from '../styles/themes/default';
import {ScrollContext} from './ScrollViewWithContext';

const propTypes = {
/** Picker label */
Expand Down Expand Up @@ -170,7 +169,7 @@ class Picker extends PureComponent {

// We add a text color to prevent white text on white background dropdown items on Windows
items={_.map(this.props.items, item => ({...item, color: themeColors.pickerOptionsTextColor}))}
style={this.props.size === 'normal' ? pickerStyles(this.props.isDisabled) : styles.pickerSmall}
style={this.props.size === 'normal' ? styles.picker(this.props.isDisabled) : styles.pickerSmall}
useNativeAndroidPickerStyle={false}
placeholder={this.placeholder}
value={this.props.value}
Expand Down
12 changes: 0 additions & 12 deletions src/components/Picker/pickerStyles/index.android.js
View file
Open in desktop

This file was deleted.

9 changes: 0 additions & 9 deletions src/components/Picker/pickerStyles/index.ios.js
View file
Open in desktop

This file was deleted.

23 changes: 0 additions & 23 deletions src/components/Picker/pickerStyles/index.js
View file
Open in desktop

This file was deleted.

71 changes: 34 additions & 37 deletions src/styles/styles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ const picker = {
fontFamily: fontFamily.EXP_NEUE,
fontSize: variables.fontSizeNormal,
lineHeight: variables.fontSizeNormalHeight,
paddingHorizontal: 11,
paddingBottom: 8,
paddingTop: 23,
paddingLeft: 0,
paddingRight: 25,
height: variables.inputHeight,
borderWidth: 0,
borderRadius: variables.componentBorderRadiusNormal,
textAlign: 'left',
};

Expand Down Expand Up @@ -560,8 +560,8 @@ const styles = {
inputIOS: {
fontFamily: fontFamily.EXP_NEUE,
fontSize: variables.fontSizeSmall,
paddingLeft: 9,
paddingRight: 25,
paddingLeft: 0,
paddingRight: 17,
paddingTop: 6,
paddingBottom: 6,
borderRadius: variables.componentBorderRadius,
Expand All @@ -574,8 +574,8 @@ const styles = {
inputWeb: {
fontFamily: fontFamily.EXP_NEUE,
fontSize: variables.fontSizeSmall,
paddingLeft: 9,
paddingRight: 25,
paddingLeft: 0,
paddingRight: 17,
paddingTop: 6,
paddingBottom: 6,
borderWidth: 0,
Expand All @@ -590,8 +590,8 @@ const styles = {
inputAndroid: {
fontFamily: fontFamily.EXP_NEUE,
fontSize: variables.fontSizeSmall,
paddingLeft: 9,
paddingRight: 25,
paddingLeft: 0,
paddingRight: 17,
paddingTop: 6,
paddingBottom: 6,
borderWidth: 0,
Expand All @@ -602,7 +602,6 @@ const styles = {
},
iconContainer: {
top: 7,
right: 8,
...pointerEventsNone,
},
icon: {
Expand Down Expand Up @@ -718,24 +717,22 @@ const styles = {

textInputContainer: {
flex: 1,
borderRadius: variables.componentBorderRadiusNormal,
justifyContent: 'center',
height: '100%',
backgroundColor: themeColors.componentBG,
borderWidth: 1,
backgroundColor: 'transparent',
borderBottomWidth: 2,
Copy link
Contributor

@aldo-expensify aldo-expensify Feb 16, 2023

Choose a reason for hiding this comment

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

Following bugzero checklist: I think this change caused this: #14632

Thanks @situchan for pointing that out.

Copy link
Member

@parasharrajat parasharrajat Feb 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Issues with RoomInput are expected as it was missed during testing because of being behind beta.

There was one more issue found before, check last comment before this.

Copy link
Contributor

@aldo-expensify aldo-expensify Feb 17, 2023

Choose a reason for hiding this comment

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

Makes sense :)

borderColor: themeColors.border,
overflow: 'hidden',
},

textInputLabel: {
position: 'absolute',
left: 11,
left: 0,
top: 0,
fontSize: variables.fontSizeNormal,
color: themeColors.textSupporting,
fontFamily: fontFamily.EXP_NEUE,
width: '100%',
textAlign: 'left',
Copy link
Member

@parasharrajat parasharrajat Jan 18, 2023

Choose a reason for hiding this comment

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

Please revert this change it will fix #12447 (review).

And we can put back the padding: 0 on textinput.

Copy link
Member

@parasharrajat parasharrajat Jan 18, 2023

Choose a reason for hiding this comment

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

@Luke9389 Can you test it?

Copy link
Contributor Author

@Luke9389 Luke9389 Jan 18, 2023

Choose a reason for hiding this comment

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

Are you sure it fixes that problem? Testing takes abt an hour to build now, so I'm not wanting to experiment on changes.

Copy link
Contributor

@0xmiroslav 0xmiroslav Jan 18, 2023

Choose a reason for hiding this comment

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

Let me confirm fix. I am able to test real time on android physical device

Copy link
Member

@parasharrajat parasharrajat Jan 18, 2023

Choose a reason for hiding this comment

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

I was thinking you will test it locally. 😄 It fixed the issue for me.

Copy link
Contributor Author

@Luke9389 Luke9389 Jan 18, 2023

Choose a reason for hiding this comment

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

Ok, there is another similar issue. It does not work when I go back to the login page and now both email and password are not accepting focus.

@parasharrajat is that after putting textAlign back in or with the code in its current state?

Copy link
Contributor Author

@Luke9389 Luke9389 Jan 18, 2023

Choose a reason for hiding this comment

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

I was thinking you will test it locally. 😄 It fixed the issue for me.

This bug doesn't occur on my emulator at all, so I need to test on a physical device (my phone). That's why I'm testing with external builds.

Copy link
Contributor

@0xmiroslav 0xmiroslav Jan 18, 2023

Choose a reason for hiding this comment

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

Still not working for me even after putting textAlign back, as long as paddingHorizontal: 0 exists

Copy link
Contributor Author

@Luke9389 Luke9389 Jan 18, 2023

Choose a reason for hiding this comment

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

The current code doesn't have textAlign and also doesn't have paddingHorizontal: 0 set. Would you be willing to test with the current code? I think it should be fixed (it's working on my phone and emulator now).

Copy link
Contributor

@0xmiroslav 0xmiroslav Jan 19, 2023

Choose a reason for hiding this comment

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

yes, current code works

},

textInputLabelBackground: {
Expand All @@ -744,8 +741,6 @@ const styles = {
width: '100%',
height: 25,
backgroundColor: themeColors.componentBG,
borderTopRightRadius: variables.componentBorderRadiusNormal,
borderTopLeftRadius: variables.componentBorderRadiusNormal,
},

textInputLabelDesktop: {
Expand All @@ -767,7 +762,7 @@ const styles = {
color: themeColors.text,
paddingTop: 23,
paddingBottom: 8,
paddingHorizontal: 11,
paddingLeft: 0,
borderWidth: 0,
},

Expand All @@ -789,8 +784,6 @@ const styles = {
textInputDesktop: addOutlineWidth({}, 0),

secureInputShowPasswordButton: {
borderTopRightRadius: 6,
borderBottomRightRadius: 6,
paddingHorizontal: 11,
justifyContent: 'center',
margin: 1,
Expand All @@ -802,7 +795,7 @@ const styles = {
},

textInput: {
backgroundColor: themeColors.componentBG,
backgroundColor: 'transparent',
borderRadius: variables.componentBorderRadiusNormal,
height: variables.inputComponentSizeNormal,
borderColor: themeColors.border,
Expand Down Expand Up @@ -832,30 +825,43 @@ const styles = {
},

pickerContainer: {
borderWidth: 1,
borderBottomWidth: 2,
paddingLeft: 0,
borderStyle: 'solid',
borderColor: themeColors.border,
borderRadius: variables.componentBorderRadiusNormal,
justifyContent: 'center',
backgroundColor: themeColors.componentBG,
backgroundColor: 'transparent',
height: variables.inputHeight,
},

pickerContainerSmall: {
height: variables.inputHeightSmall,
},

pickerLabel: {
position: 'absolute',
left: 11,
left: 0,
top: 6,
},

picker: (disabled = false) => ({
iconContainer: {
top: Math.round(variables.inputHeight * 0.5) - 11,
right: 10,
right: 0,
zIndex: -1,
},

inputWeb: {
appearance: 'none',
cursor: disabled ? 'not-allowed' : 'pointer',
...picker,
},
inputNative: {

inputIOS: {
...picker,
},

inputAndroid: {
...picker,
},
}),
Expand All @@ -869,16 +875,6 @@ const styles = {
color: themeColors.icon,
},

textInputReversed: addOutlineWidth({
backgroundColor: themeColors.heading,
borderColor: themeColors.text,
color: themeColors.textReversed,
}, 0),

textInputReversedFocus: {
borderColor: themeColors.icon,
},

noOutline: addOutlineWidth({}, 0),

errorOutline: {
Expand Down Expand Up @@ -1403,7 +1399,6 @@ const styles = {
fontFamily: fontFamily.EMOJI_TEXT_FONT,
fontSize: variables.fontSizeNormal,
borderWidth: 0,
borderRadius: variables.componentBorderRadiusRounded,
height: 'auto',
lineHeight: variables.lineHeightXLarge,
...overflowXHidden,
Expand All @@ -1426,6 +1421,7 @@ const styles = {
},

editInputComposeSpacing: {
backgroundColor: themeColors.transparent,
marginVertical: 6,
},

Expand Down Expand Up @@ -1813,6 +1809,7 @@ const styles = {

borderNone: {
borderWidth: 0,
borderBottomWidth: 0,
},

borderRight: {
Expand Down
3 changes: 2 additions & 1 deletion src/styles/variables.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ export default {
lineHeightNormal: getValueUsingPixelRatio(16, 21),
lineHeightLarge: getValueUsingPixelRatio(18, 24),
lineHeightXLarge: getValueUsingPixelRatio(20, 24),
inputHeight: getValueUsingPixelRatio(50, 72),
inputHeight: getValueUsingPixelRatio(52, 72),
inputHeightSmall: 28,
formErrorLineHeight: getValueUsingPixelRatio(18, 23),
communicationsLinkHeight: getValueUsingPixelRatio(20, 30),
alternateTextHeight: getValueUsingPixelRatio(20, 24),
Expand Down