-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix: paddingHorizontal with TextInput.Affix #2312
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
Conversation
@Trancever @unikvozm @jbinda @matkoson @OlimpiaZurek Please check. Thanks. |
@@ -29,6 +29,7 @@ type ContextState = { | |||
visible?: Animated.Value; | |||
textStyle?: StyleProp<TextStyle>; | |||
side: AdornmentSide; | |||
paddingHorizontal?: string | number | undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we type it as paddingHorizontal?: number;
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure but I get Type 'string | number | undefined' is not assignable to type 'number'.
error if I use paddingHorizontal?: number;
. I am also new to typescript, any pointers will be appreciated.
Also according to the docs, paddingHorizontal
has a type of number, string
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I see. Then it should be paddingHorizontal?: number | string
. We don't have to add undefined
, because we already have a question mark after the prop name.
const textColor = color(theme.colors.text) | ||
.alpha(theme.dark ? 0.7 : 0.54) | ||
.rgb() | ||
.string(); | ||
|
||
const offset = | ||
paddingHorizontal !== undefined && typeof paddingHorizontal === 'number' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we change the type we won't need this check typeof paddingHorizontal === 'number'
inputOffset = 0, | ||
}: { | ||
inputOffset?: number; | ||
adornmentConfig: AdornmentConfig[]; | ||
leftAffixWidth: number; | ||
rightAffixWidth: number; | ||
paddingHorizontal?: string | number | undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
@@ -114,6 +121,7 @@ export interface TextInputAdornmentProps { | |||
textStyle?: StyleProp<TextStyle>; | |||
visible?: Animated.Value; | |||
isTextInputFocused: boolean; | |||
paddingHorizontal?: string | number | undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here
@Trancever Updated to |
Merged in 4b1c150 |
Does TextInput.Icon support? |
Summary
Fixes: #2303
paddingHorizontal
is disregarded when usingleft
prop onTextInput
.Test plan
Snack: https://snack.expo.io/@dcangulo/textinput
Sample Code in screenshot:
Before this PR:

After this PR:
