[TextInput] Implements `blurOnSubmit` #2149

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
@dsibiski
Contributor

dsibiski commented Jul 28, 2015

The default value (to retain current behavior) is set to true. Setting the value to false will prevent the textField from blurring but still fire the onSubmitEditing callback. However, the onEndEditing callback will not be fired.

Addresses issue: #2129

@dsibiski

This comment has been minimized.

Show comment
Hide comment
@dsibiski

dsibiski Jul 28, 2015

Contributor

Doesn't look like this current implementation is going to work as it also blocks switching to other TextInputs and is causing some strange things with CMD-R...looking into those...

Contributor

dsibiski commented Jul 28, 2015

Doesn't look like this current implementation is going to work as it also blocks switching to other TextInputs and is causing some strange things with CMD-R...looking into those...

@brentvatne

This comment has been minimized.

Show comment
Hide comment
@brentvatne

brentvatne Jul 28, 2015

Collaborator

@dsibiski - this is awesome, I think it's a very important feature to get in. Have you fixed the implementation since your previous comment?

Collaborator

brentvatne commented Jul 28, 2015

@dsibiski - this is awesome, I think it's a very important feature to get in. Have you fixed the implementation since your previous comment?

@brentvatne

This comment has been minimized.

Show comment
Hide comment
@brentvatne

brentvatne Jul 28, 2015

Collaborator

Pulling in your comment from #641

Basically, it allows for backward compatibility by defaulting to true, however, the main idea goes with the normal iOS convention that the keyboard should technically stay there even on submit. One day the default value of true could be deprecated.

In the case of switching to different fields...the user would set blurOnSubmit to false (currently) and the keyboard would not dismiss. However, the onSubmitEditing callback would still fire and they would be able to programmatically do what @ide described earlier, by simply calling focus() on the ref of their choice. No keyboard "jumping" to deal with. :)

Collaborator

brentvatne commented Jul 28, 2015

Pulling in your comment from #641

Basically, it allows for backward compatibility by defaulting to true, however, the main idea goes with the normal iOS convention that the keyboard should technically stay there even on submit. One day the default value of true could be deprecated.

In the case of switching to different fields...the user would set blurOnSubmit to false (currently) and the keyboard would not dismiss. However, the onSubmitEditing callback would still fire and they would be able to programmatically do what @ide described earlier, by simply calling focus() on the ref of their choice. No keyboard "jumping" to deal with. :)

@dsibiski

This comment has been minimized.

Show comment
Hide comment
@dsibiski

dsibiski Jul 28, 2015

Contributor

@brentvatne Yeah, my latest commit fixes the issue I commented on.

Contributor

dsibiski commented Jul 28, 2015

@brentvatne Yeah, my latest commit fixes the issue I commented on.

@brentvatne

This comment has been minimized.

Show comment
Hide comment
Collaborator

brentvatne commented Jul 28, 2015

@dsibiski

This comment has been minimized.

Show comment
Hide comment
@dsibiski

dsibiski Aug 7, 2015

Contributor

@nicklockwood Any thoughts on this one?

Contributor

dsibiski commented Aug 7, 2015

@nicklockwood Any thoughts on this one?

@jim-lake

This comment has been minimized.

Show comment
Hide comment
@jim-lake

jim-lake Aug 9, 2015

This or something similar would be great to prevent keyboard flashing in apps.

jim-lake commented Aug 9, 2015

This or something similar would be great to prevent keyboard flashing in apps.

@dsibiski

This comment has been minimized.

Show comment
Hide comment
@dsibiski

dsibiski Aug 10, 2015

Contributor

Here is a quick demo of what this looks like when its working to switch between fields in a form...

bluronsubmit_demo

Here is the sample code that makes this work:

var InputScreen = React.createClass({
    _focusNextField(nextField) {
        this.refs[nextField].focus()
    },

    render: function() {
        return (
            <View style={styles.container}>
                <TextInput
                    ref='1'
                    style={styles.input}
                    placeholder='Normal'
                    returnKeyType='next'
                    blurOnSubmit={false}
                    onSubmitEditing={() => this._focusNextField('2')}
                />
                <TextInput
                    ref='2'
                    style={styles.input}
                    keyboardType='email-address'
                    placeholder='Email Address'
                    returnKeyType='next'
                    blurOnSubmit={false}
                    onSubmitEditing={() => this._focusNextField('3')}
                />
                <TextInput
                    ref='3'
                    style={styles.input}
                    keyboardType='url'
                    placeholder='URL'
                    returnKeyType='next'
                    blurOnSubmit={false}
                    onSubmitEditing={() => this._focusNextField('4')}
                />
                <TextInput
                    ref='4'
                    style={styles.input}
                    keyboardType='numeric'
                    placeholder='Numeric'
                    blurOnSubmit={false}
                    onSubmitEditing={() => this._focusNextField('5')}
                />
                <TextInput
                    ref='5'
                    style={styles.input}
                    keyboardType='numbers-and-punctuation'
                    placeholder='Numbers & Punctuation'
                    returnKeyType='done'
                />
            </View>
        );
    }
});
Contributor

dsibiski commented Aug 10, 2015

Here is a quick demo of what this looks like when its working to switch between fields in a form...

bluronsubmit_demo

Here is the sample code that makes this work:

var InputScreen = React.createClass({
    _focusNextField(nextField) {
        this.refs[nextField].focus()
    },

    render: function() {
        return (
            <View style={styles.container}>
                <TextInput
                    ref='1'
                    style={styles.input}
                    placeholder='Normal'
                    returnKeyType='next'
                    blurOnSubmit={false}
                    onSubmitEditing={() => this._focusNextField('2')}
                />
                <TextInput
                    ref='2'
                    style={styles.input}
                    keyboardType='email-address'
                    placeholder='Email Address'
                    returnKeyType='next'
                    blurOnSubmit={false}
                    onSubmitEditing={() => this._focusNextField('3')}
                />
                <TextInput
                    ref='3'
                    style={styles.input}
                    keyboardType='url'
                    placeholder='URL'
                    returnKeyType='next'
                    blurOnSubmit={false}
                    onSubmitEditing={() => this._focusNextField('4')}
                />
                <TextInput
                    ref='4'
                    style={styles.input}
                    keyboardType='numeric'
                    placeholder='Numeric'
                    blurOnSubmit={false}
                    onSubmitEditing={() => this._focusNextField('5')}
                />
                <TextInput
                    ref='5'
                    style={styles.input}
                    keyboardType='numbers-and-punctuation'
                    placeholder='Numbers & Punctuation'
                    returnKeyType='done'
                />
            </View>
        );
    }
});
@PhilippKrone

This comment has been minimized.

Show comment
Hide comment
@PhilippKrone

PhilippKrone Sep 13, 2015

Contributor

cc @nicklockwood @sahrens Any idea when this is going to get merged?

Contributor

PhilippKrone commented Sep 13, 2015

cc @nicklockwood @sahrens Any idea when this is going to get merged?

@PhilippKrone

This comment has been minimized.

Show comment
Hide comment
@PhilippKrone

PhilippKrone Sep 13, 2015

Contributor

@dsibiski I've just applied your pull request into my fork (which is basically facebook/react-native master) and its not working. During debugging, I've noticed that its not jumping into

- (BOOL)textFieldShouldEndEditing:(UITextField *)textField
{
  if (_submitted) {
    _submitted = NO;
    return _blurOnSubmit;
  }
  return YES;
}

at all.

Do you have any idea here?

Contributor

PhilippKrone commented Sep 13, 2015

@dsibiski I've just applied your pull request into my fork (which is basically facebook/react-native master) and its not working. During debugging, I've noticed that its not jumping into

- (BOOL)textFieldShouldEndEditing:(UITextField *)textField
{
  if (_submitted) {
    _submitted = NO;
    return _blurOnSubmit;
  }
  return YES;
}

at all.

Do you have any idea here?

@dsibiski

This comment has been minimized.

Show comment
Hide comment
@dsibiski

dsibiski Sep 13, 2015

Contributor

@PhilipKrone Ah yes, the delegate was moved to the RCTTextFieldManager. Should be able to move this implementation there with some slight modification. I'll try to do this later today.

Contributor

dsibiski commented Sep 13, 2015

@PhilipKrone Ah yes, the delegate was moved to the RCTTextFieldManager. Should be able to move this implementation there with some slight modification. I'll try to do this later today.

@dsibiski

This comment has been minimized.

Show comment
Hide comment
@dsibiski

dsibiski Sep 13, 2015

Contributor

@PhilippKrone That should work now. Please let me know.

Contributor

dsibiski commented Sep 13, 2015

@PhilippKrone That should work now. Please let me know.

@PhilippKrone

This comment has been minimized.

Show comment
Hide comment
@PhilippKrone

PhilippKrone Sep 13, 2015

Contributor

Perfect, thanks a lot - it’s working. Would love to see it in core.

On 13 Sep 2015, at 18:43, Dave Sibiski notifications@github.com wrote:

@PhilippKrone https://github.com/PhilippKrone That should work now. Please let me know.


Reply to this email directly or view it on GitHub #2149 (comment).

Contributor

PhilippKrone commented Sep 13, 2015

Perfect, thanks a lot - it’s working. Would love to see it in core.

On 13 Sep 2015, at 18:43, Dave Sibiski notifications@github.com wrote:

@PhilippKrone https://github.com/PhilippKrone That should work now. Please let me know.


Reply to this email directly or view it on GitHub #2149 (comment).

@JakeRawr

This comment has been minimized.

Show comment
Hide comment
@JakeRawr

JakeRawr Sep 13, 2015

Contributor

Thanks, @dsibiski

Before your work, my message system is not as user friendly because when they send a message, they had to refocus the textinput to show the keyboard.

Now the message chat is working like how it should be.

I hope this will be merged soon.

Contributor

JakeRawr commented Sep 13, 2015

Thanks, @dsibiski

Before your work, my message system is not as user friendly because when they send a message, they had to refocus the textinput to show the keyboard.

Now the message chat is working like how it should be.

I hope this will be merged soon.

@brentvatne

This comment has been minimized.

Show comment
Hide comment
@brentvatne

brentvatne Sep 13, 2015

Collaborator

Awesome @dsibiski

@nicklockwood @sahrens - I'd love to see this get imported when you get a chance

Collaborator

brentvatne commented Sep 13, 2015

Awesome @dsibiski

@nicklockwood @sahrens - I'd love to see this get imported when you get a chance

@cgarvis

This comment has been minimized.

Show comment
Hide comment

cgarvis commented Sep 25, 2015

👍

@morenoh149

This comment has been minimized.

Show comment
Hide comment
@morenoh149

morenoh149 Oct 30, 2015

Contributor

would be useful to me too

Contributor

morenoh149 commented Oct 30, 2015

would be useful to me too

dsibiski added some commits Jul 28, 2015

[TextInput] Implements `blurOnSubmit`
The default value (to retain current behavior) is set to `true`. Setting the value to `false` will prevent the textField from blurring but still fire the `onSubmitEditing` callback. However, the `onEndEditing` callback will not be fired.

Addresses issue: #2129
Fixes implementation to only block the blur on `submit`, not when the…
…y are simply switching or blurring normally.
Passes the `textFieldShouldEndEditing` delegate method to the RCTText…
…Field

- Instead of implementing it in the manager, we just pass through to save exposing implementation details in RCTTextField to it's public interface (For example: The local variable `_summited` would have had to be a public property)
@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Nov 4, 2015

@dsibiski updated the pull request.

@dsibiski updated the pull request.

@dsibiski

This comment has been minimized.

Show comment
Hide comment
@dsibiski

dsibiski Nov 4, 2015

Contributor

@nicklockwood I think the last bit left to be able to push this one out would be adding a UIExplorer example and squashing commits...anything else?

Contributor

dsibiski commented Nov 4, 2015

@nicklockwood I think the last bit left to be able to push this one out would be adding a UIExplorer example and squashing commits...anything else?

@nicklockwood

This comment has been minimized.

Show comment
Hide comment
@nicklockwood

nicklockwood Nov 4, 2015

Contributor

A UIExplorer example would be great. Don't worry about squashing commits.

Contributor

nicklockwood commented Nov 4, 2015

A UIExplorer example would be great. Don't worry about squashing commits.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Nov 5, 2015

@dsibiski updated the pull request.

@dsibiski updated the pull request.

@nicklockwood

This comment has been minimized.

Show comment
Hide comment
Contributor

nicklockwood commented Nov 5, 2015

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment

@ghost ghost closed this in 7af7524 Nov 5, 2015

@ajwhite

This comment has been minimized.

Show comment
Hide comment
@ajwhite

ajwhite Nov 5, 2015

Contributor

👏 great enhancement

Contributor

ajwhite commented Nov 5, 2015

👏 great enhancement

MattFoley added a commit to skillz/react-native that referenced this pull request Nov 9, 2015

Implements `blurOnSubmit`
Summary: The default value (to retain current behavior) is set to `true`. Setting the value to `false` will prevent the textField from blurring but still fire the `onSubmitEditing` callback. However, the `onEndEditing` callback will not be fired.

Addresses issue: facebook#2129
Closes facebook#2149

Reviewed By: svcscm

Differential Revision: D2619822

Pulled By: nicklockwood

fb-gh-sync-id: 9a61152892f4afb5c6c53e7b38dffae13bc7e13f

calvinf pushed a commit to calvinf/react-native that referenced this pull request Nov 18, 2015

Implements `blurOnSubmit`
Summary: The default value (to retain current behavior) is set to `true`. Setting the value to `false` will prevent the textField from blurring but still fire the `onSubmitEditing` callback. However, the `onEndEditing` callback will not be fired.

Addresses issue: facebook#2129
Closes facebook#2149

Reviewed By: svcscm

Differential Revision: D2619822

Pulled By: nicklockwood

fb-gh-sync-id: 9a61152892f4afb5c6c53e7b38dffae13bc7e13f
@ruandao

This comment has been minimized.

Show comment
Hide comment
@ruandao

ruandao Nov 24, 2015

Execute me!
Where to find the focus function, it work correct, but I can't found it at TextInput Props, I mean where to find some other property like focus
Thank you!

ruandao commented Nov 24, 2015

Execute me!
Where to find the focus function, it work correct, but I can't found it at TextInput Props, I mean where to find some other property like focus
Thank you!

@dsibiski

This comment has been minimized.

Show comment
Hide comment
@dsibiski

dsibiski Nov 25, 2015

Contributor

@ruandao: The focus() method is actually defined in NativeMethodsMixin here:

/**
* Requests focus for the given input or view. The exact behavior triggered
* will depend on the platform and type of view.
*/
focus: function() {
TextInputState.focusTextInput(findNodeHandle(this));
},

Contributor

dsibiski commented Nov 25, 2015

@ruandao: The focus() method is actually defined in NativeMethodsMixin here:

/**
* Requests focus for the given input or view. The exact behavior triggered
* will depend on the platform and type of view.
*/
focus: function() {
TextInputState.focusTextInput(findNodeHandle(this));
},

trabianmatt added a commit to trabian/react-native that referenced this pull request Nov 27, 2015

Implements `blurOnSubmit`
Summary: The default value (to retain current behavior) is set to `true`. Setting the value to `false` will prevent the textField from blurring but still fire the `onSubmitEditing` callback. However, the `onEndEditing` callback will not be fired.

Addresses issue: facebook#2129
Closes facebook#2149

Reviewed By: svcscm

Differential Revision: D2619822

Pulled By: nicklockwood

fb-gh-sync-id: 9a61152892f4afb5c6c53e7b38dffae13bc7e13f

Conflicts:
	Libraries/Text/RCTTextField.h
	Libraries/Text/RCTTextFieldManager.m
@ehd

This comment has been minimized.

Show comment
Hide comment
@ehd

ehd Dec 1, 2015

Hey, this is a feature I was really looking forward to, thanks for adding it!

When I played with this I noticed that it's implemented for UITextField, but not for a multiline UITextView. Is that something we want to support? UITextViewDelegate does have an accompanying method.

ehd commented Dec 1, 2015

Hey, this is a feature I was really looking forward to, thanks for adding it!

When I played with this I noticed that it's implemented for UITextField, but not for a multiline UITextView. Is that something we want to support? UITextViewDelegate does have an accompanying method.

@chirag04

This comment has been minimized.

Show comment
Hide comment
@chirag04

chirag04 Dec 1, 2015

Collaborator

@ehd can you PR?

Collaborator

chirag04 commented Dec 1, 2015

@ehd can you PR?

@nicklockwood

This comment has been minimized.

Show comment
Hide comment
@nicklockwood

nicklockwood Dec 1, 2015

Contributor

The reason we don't have this for UITextView is that it treats <return> differently from UITextField. For UITextField, return characters are ignored, and pressing return either does nothing or ends editing and submits the field, but for UITextView, return is a valid character that will be inserted into the text.

If we were going to add blurOnSubmit for multiline fields it would have to default to false to avoid changing the default behaviour. I guess we can make that work though.

The alternative would be to have a separate property like submitOnReturn, but I'm not so keen on that (and it would still need to have a different default for single vs multiline so it doesn't really solve anything).

Contributor

nicklockwood commented Dec 1, 2015

The reason we don't have this for UITextView is that it treats <return> differently from UITextField. For UITextField, return characters are ignored, and pressing return either does nothing or ends editing and submits the field, but for UITextView, return is a valid character that will be inserted into the text.

If we were going to add blurOnSubmit for multiline fields it would have to default to false to avoid changing the default behaviour. I guess we can make that work though.

The alternative would be to have a separate property like submitOnReturn, but I'm not so keen on that (and it would still need to have a different default for single vs multiline so it doesn't really solve anything).

@ehd

This comment has been minimized.

Show comment
Hide comment
@ehd

ehd Dec 1, 2015

Yes, I now see what you mean after experimenting with the view's current behavior in v0.16.0-rc. In my case, in the multiline text input I was actually calling blur and have forgotten about it. So I'm just clearing the text now without bluring. However, I've been unable to handle the enter key the way I want to (to not create a newline, but to call a callback) without causing a value change event to be triggered, too.

ehd commented Dec 1, 2015

Yes, I now see what you mean after experimenting with the view's current behavior in v0.16.0-rc. In my case, in the multiline text input I was actually calling blur and have forgotten about it. So I'm just clearing the text now without bluring. However, I've been unable to handle the enter key the way I want to (to not create a newline, but to call a callback) without causing a value change event to be triggered, too.

@nicklockwood

This comment has been minimized.

Show comment
Hide comment
@nicklockwood

nicklockwood Dec 1, 2015

Contributor

I'm going to try adding blurOnSubmit support for multiline textInput. I'll let you know how it goes.

Contributor

nicklockwood commented Dec 1, 2015

I'm going to try adding blurOnSubmit support for multiline textInput. I'll let you know how it goes.

@ehd

This comment has been minimized.

Show comment
Hide comment
@ehd

ehd Dec 1, 2015

@nicklockwood That is great, thank you!

ehd commented Dec 1, 2015

@nicklockwood That is great, thank you!

@nicklockwood

This comment has been minimized.

Show comment
Hide comment
@nicklockwood

nicklockwood Dec 1, 2015

Contributor

I've realised that there's a slight problem with this plan. blurOnSubmit = true will work fine for multiline fields, but blurOnSubmit = false (the default) won't actually do what the blurOnSubmit was originally intended for, namely allowing you to implement a multi-field form without keyboard flicker.

I don't think this is a reason not to ship the feature, but if anybody wants a multiline text field where pressing return triggers the submit action without blurring the field, we'll have to think of something else.

Contributor

nicklockwood commented Dec 1, 2015

I've realised that there's a slight problem with this plan. blurOnSubmit = true will work fine for multiline fields, but blurOnSubmit = false (the default) won't actually do what the blurOnSubmit was originally intended for, namely allowing you to implement a multi-field form without keyboard flicker.

I don't think this is a reason not to ship the feature, but if anybody wants a multiline text field where pressing return triggers the submit action without blurring the field, we'll have to think of something else.

nicklockwood added a commit that referenced this pull request Dec 2, 2015

Added blurOnSubmit support to multine TextInput (aka RCTTextView)
Summary:
public

Setting `blurOnSubmit=true` on a multiline `<TextInput>` now causes it to behave like a single-line input with respect to the return key:

With the default value of `false`, pressing return will enter a newline character into the field. If you set the value to `true`, pressing return will now blur the field and trigger the onSubmitEditing event. The newline character will *not* be added to the text.

(See associated github task for dicussion: #2149)

Reviewed By: javache

Differential Revision: D2710448

fb-gh-sync-id: c9706ae11f8b399932d3400ceb4c7558e455570d

Crash-- added a commit to Crash--/react-native that referenced this pull request Dec 24, 2015

Implements `blurOnSubmit`
Summary: The default value (to retain current behavior) is set to `true`. Setting the value to `false` will prevent the textField from blurring but still fire the `onSubmitEditing` callback. However, the `onEndEditing` callback will not be fired.

Addresses issue: facebook#2129
Closes facebook#2149

Reviewed By: svcscm

Differential Revision: D2619822

Pulled By: nicklockwood

fb-gh-sync-id: 9a61152892f4afb5c6c53e7b38dffae13bc7e13f

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment