-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
feat: Add inputMode prop to TextInput component #34460
feat: Add inputMode prop to TextInput component #34460
Conversation
Base commit: 8c882b4 |
Base commit: 8c882b4 |
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
'none' => 'default' (or noop) sounds right. |
| 'tel' | ||
| 'search' | ||
| 'email' | ||
| 'url'; |
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.
'none' should be added to the allowed values even if we do nothing with it
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.
Sounds good, I've just added none
there and mapped it to default
11fb0b7
to
e5f7e1a
Compare
Doing the same thing as Chrome seems reasonable. So |
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
e5f7e1a
to
69f4b33
Compare
@cipolleschi I've just rebased this now that CI has been fixed |
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
69f4b33
to
20fe307
Compare
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@necolas @cipolleschi any thoughts on why this didn't get labeled as |
Apparently inputMode=none could be mapped to |
Sounds good to me, gonna open a follow-up PR updating this |
|
#35228) Summary: This PR updates `inputMode` prop from the `TextInput` component to map the `none` option to `showSoftInputOnFocus={false}` as suggested by necolas here -> #34460 (comment). This change makes the inputMode API behaves a bit more similarly across platforms. Related to necolas/react-native-web#2421 ## Changelog [General] [Changed] - Update TextInput inputMode to map "none" to showSoftInputOnFocus ## Test Plan 1. Open the RNTester app and navigate to the TextInput page 2. Test the `TextInput` component through the `Input modes` section https://user-images.githubusercontent.com/11707729/200218435-6a33b319-e989-4086-aac3-506546982b38.mov Pull Request resolved: #35228 Reviewed By: lunaleaps, necolas Differential Revision: D41081876 Pulled By: jacdebug fbshipit-source-id: cc634c3723647d8950bf2cfe67be70d0fbd488a6
facebook#35228) Summary: This PR updates `inputMode` prop from the `TextInput` component to map the `none` option to `showSoftInputOnFocus={false}` as suggested by necolas here -> facebook#34460 (comment). This change makes the inputMode API behaves a bit more similarly across platforms. Related to necolas/react-native-web#2421 ## Changelog [General] [Changed] - Update TextInput inputMode to map "none" to showSoftInputOnFocus ## Test Plan 1. Open the RNTester app and navigate to the TextInput page 2. Test the `TextInput` component through the `Input modes` section https://user-images.githubusercontent.com/11707729/200218435-6a33b319-e989-4086-aac3-506546982b38.mov Pull Request resolved: facebook#35228 Reviewed By: lunaleaps, necolas Differential Revision: D41081876 Pulled By: jacdebug fbshipit-source-id: cc634c3723647d8950bf2cfe67be70d0fbd488a6
Summary
This adds the
inputMode
prop to the TextInput component as requested on #34424, mapping web inputMode types to equivalent keyboardType values. This PR also updates RNTester TextInputExample in order to facilitate the manual QA.Caveats
This only adds support totext
,decimal
,numeric
,tel
,search
,email
, andurl
types.inputMode="none"
Currently mapped to
default
keyboard type.The main problem with this input mode is that it's not supported natively neither on Android or iOS. Android
TextView
does acceptnone
asandroid:inputType
but that makes the text not editable, which wouldn't really solve our problem.UITextInput
on iOS on the other hand doesn't even have something similar to avoid displaying the virtual keyboard.If we really want to add the support for
inputMode="none"
one interesting approach we could take is to do something similar to what WebKit has done (WebKit/WebKit@3b5f0c8). In order to achieve this behavior, they had to return aUIView
with a bounds ofCGRectZero
as the inputView of theWKContentView
when inputmode=none is present.I guess the real question here should be, do we really want to add this? Or perhaps should we just mapinputMode="none"
tokeyboardType="default"
Android docs: https://developer.android.com/reference/android/widget/TextView#attr_android:inputType
iOS docs: https://developer.apple.com/documentation/uikit/uikeyboardtype?language=objc
inputMode="search"
on AndroidCurrently mapped to
default
keyboard type.Android
TextView
does not offers any options likeUIKeyboardTypeWebSearch
on iOS to be used assearch
withandroid:inputType
and that's probably the reason whykeyboardType="web-search"
is iOS only. I checked how this is handled on the browser on my Android device and it seems that Chrome just uses the default keyboard, maybe we should do the same?Open questions
What should be done aboutAdd it and map it toinputMode="none"
?default
keyboard type.Which keyboard should we show on Android whenUse theinputMode="search"
?default
keyboard the same way Chrome doesChangelog
[General] [Added] - Add inputMode prop to TextInput component
Test Plan
TextInput
component through theInput modes
sectionScreen.Recording.2022-08-19.at.16.16.10.mov