-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Split TextInput into TextInputOutlined and TextInputFlat #882
Conversation
Hey @pbitkowski, thank you for your pull request 🤗. The documentation from this branch can be viewed here. Please remember to update Typescript types if you changed API. |
I have an issue with Flow here. It logs a long list of errors connected to possible usage of TextInput (from React Native) props on our TextInput. I used the same Properties as before so it's weird that it doesn't work. I'm not sure if the intersaction of |
I think the state can be moved up to the For flow types, try spreading them instead of intersection: type Props = {|
...React.ElementConfig<typeof NativeTextInput>,
|} |
0e25536
to
9af5550
Compare
@satya164 I moved types to common file and used spread operator for extending React Native TextInput Props but still got errors, which is weird. BTW. I rebased my branch to master and some TS errors appeared. |
9af5550
to
b48b62f
Compare
Fixed flow errors and rebased. Could you take a look @Trancever ? |
error: false, | ||
multiline: false, | ||
editable: true, | ||
render: props => <NativeTextInput {...props} />, |
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.
Shouldn't we type it as RenderProps just like in other components?
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.
Done
|
||
return ( | ||
<View style={[containerStyle, style]}> | ||
{/* When mode === 'flat', render an underline */} |
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.
We can remove this
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.
Done
|
||
{mode === 'outlined' && label ? ( | ||
{label ? ( | ||
// When mode == 'outlined', the input label stays on top of the outline |
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.
We can remove When mode == 'outlined
from comment :D
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.
Done
b72cc59
to
7a905e7
Compare
- make TextInput proxy component that renders TextInputOutlined or TextInputFlat based on 'mode' props, - move TextInput components to its own directory and change imports in index to make it work.
- move types from TextInput, TextInputFlat, TextInputOutlined to common file, - remove documentation from TextInputFlat and TextInputOutlined
7a905e7
to
f8fc90c
Compare
What this PR consists of:
Motivation
Issue #791
Test plan
Open example and compare it with version without my changes.