-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Make PlainInput controllable #13795
Make PlainInput controllable #13795
Changes from 12 commits
13b6192
4004de4
fa109e5
7e6e920
b0a3344
93471da
f5ce402
3f33463
bb6a720
309eaa3
820d9ca
368bc23
d5862e4
537429f
05d5357
e80be0e
bbc8dc8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ export type KeyboardType = | |
// Android Only | ||
| 'visible-password' | ||
|
||
export type Props = { | ||
export type Props = {| | ||
autoFocus?: boolean, | ||
className?: string, | ||
disabled?: boolean, | ||
|
@@ -38,6 +38,7 @@ export type Props = { | |
style?: StylesCrossPlatform, | ||
textType?: TextType, | ||
type?: 'password' | 'text' | 'number', | ||
value?: string, // Makes this a controlled input when passed. Also disables mutating value via `transformText`, see note at component API | ||
|
||
/* Platform discrepancies */ | ||
// Maps to onSubmitEditing on native | ||
|
@@ -55,7 +56,7 @@ export type Props = { | |
returnKeyType?: 'done' | 'go' | 'next' | 'search' | 'send', | ||
selectTextOnFocus?: boolean, | ||
onEndEditing?: () => void, | ||
} | ||
|} | ||
|
||
// Use this to mix your props with input props like type Props = PropsWithInput<{foo: number}> | ||
export type PropsWithInput<P> = {| | ||
|
@@ -73,10 +74,10 @@ export type PropsWithInput<P> = {| | |
* use `InternalProps`. | ||
* See more discussion here: https://github.com/facebook/flow/issues/1660 | ||
*/ | ||
export type DefaultProps = { | ||
export type DefaultProps = {| | ||
keyboardType: KeyboardType, | ||
textType: TextType, | ||
} | ||
|} | ||
|
||
export type Selection = {start: number, end: number} | ||
|
||
|
@@ -85,11 +86,24 @@ export type TextInfo = { | |
selection: Selection, | ||
} | ||
|
||
export type InternalProps = DefaultProps & Props | ||
export type InternalProps = {...DefaultProps, ...Props} | ||
declare export default class PlainInput extends React.Component<Props> { | ||
static defaultProps: DefaultProps; | ||
blur: () => void; | ||
focus: () => void; | ||
// Supported only on desktop right now | ||
/** | ||
* This can only be used when the input is controlled. Use `transformText` if | ||
* you want to do this on an uncontrolled input. Make sure the Selection is | ||
* valid against the `value` prop. Avoid changing `value` and calling this at | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm...so now I'm confused by this API. We almost always want to change both the text and the selection at the same time...AIUI the existing use cases are inserting text (like emoji, mentions, etc.) at the cursor. So that's why I suggested having a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I ran into issues early on with the parent actively managing the selection and value at the same time. AFAICT on desktop there's no way to listen to events that change the cursor position (without selecting anything). Without the parent knowing the selection beforehand, I think the semantics of the prop would be weird, and even then I'm not sure if there's a way to insert text at the cursor the way we want. I'm beginning to think having a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, yes, I remember running into that when I tried. @chrisnojima did mention there was some way to listen to all DOM events and then filter out the selection ones on a given component, but I didn't know enough to pursue that. ...and then, yeah, you can try implementing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @akalin-keybase OK, rather than going down the murky source of truth route, I added a |
||
* the same time if you don't want bad things to happen. Note that a | ||
* selection will only appear when the input is focused. Call `focus()` | ||
* before this if you want to be sure the user will see the selection. | ||
**/ | ||
setSelection: Selection => void; | ||
/** | ||
* This can only be used when the input is uncontrolled. Like `setSelection`, | ||
* if you want to be sure the user will see a selection use `focus()` before | ||
* calling this. | ||
**/ | ||
transformText: (fn: (TextInfo) => TextInfo, reflectChange?: boolean) => void; | ||
} |
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 you check if it's null or undefined instead? since empty string can be a valid value.
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.
yup, thanks!