[Blur & Focus] Support relinquishing focus from multiple views via co… #2203

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
5 participants
@josebalius

…ntainer - #113

This adds the ability to call this.refs.parentView.blur() and blur any active text fields inside the view. It will first try to blur the field (already supported) and if the field is not found as an "active" text field, it will transverse the view and resignFirstResponder for each of them.

I am currently using it in my app and works well.

React/Modules/RCTUIManager.m
+ [view resignFirstResponder];
+ }
+ }
+}

This comment has been minimized.

@ide

ide Aug 2, 2015

Collaborator

You should be able to do something like [subview endEditing:YES] instead of doing the complete traversal. Also can remove the uiManager parameter since self == uiManager.

UIView *view = uiManager.viewRegistry[reactTag];
if (view) {
  [view endEditing:YES];
} else {
  // Recurse through the shadow hierarchy when the shadow view has no backing UIView
  for (RCTShadowView *subview in [shadowView reactSubviews]) {
    [self endEditingForShadowView:subview.reactTag viewRegistry:viewRegistry];
  }
}
@ide

ide Aug 2, 2015

Collaborator

You should be able to do something like [subview endEditing:YES] instead of doing the complete traversal. Also can remove the uiManager parameter since self == uiManager.

UIView *view = uiManager.viewRegistry[reactTag];
if (view) {
  [view endEditing:YES];
} else {
  // Recurse through the shadow hierarchy when the shadow view has no backing UIView
  for (RCTShadowView *subview in [shadowView reactSubviews]) {
    [self endEditingForShadowView:subview.reactTag viewRegistry:viewRegistry];
  }
}

This comment has been minimized.

@josebalius

josebalius Aug 2, 2015

@ide Makes perfect sense, i'll make the changes, test and commit.

@josebalius

josebalius Aug 2, 2015

@ide Makes perfect sense, i'll make the changes, test and commit.

- Removing unnecessary parameters of uiManager and viewRegistry.
- Using endEditing vs resignFirstResponder as it is more efficient.
@josebalius

This comment has been minimized.

Show comment
Hide comment
@josebalius

josebalius Aug 2, 2015

@ide Made the changes you requested and tested them in my app and everything seems to work great, let me know if you think anything else is needed.

@ide Made the changes you requested and tested them in my app and everything seems to work great, let me know if you think anything else is needed.

@brentvatne

This comment has been minimized.

Show comment
Hide comment
@brentvatne

brentvatne Aug 2, 2015

Collaborator

I like this 😄 👍

Collaborator

brentvatne commented Aug 2, 2015

I like this 😄 👍

@josebalius

This comment has been minimized.

Show comment
Hide comment
@josebalius

josebalius Aug 12, 2015

@brentvatne not sure what went wrong with the build?

@brentvatne not sure what went wrong with the build?

@brentvatne

This comment has been minimized.

Show comment
Hide comment
@brentvatne

brentvatne Jan 3, 2016

Collaborator

@josebalius - if #113 works (assuming we can resolve the issue you mentioned in #113 (comment)) is this still needed?

Collaborator

brentvatne commented Jan 3, 2016

@josebalius - if #113 works (assuming we can resolve the issue you mentioned in #113 (comment)) is this still needed?

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Jan 3, 2016

@josebalius updated the pull request.

@josebalius updated the pull request.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Mar 3, 2016

@spicyj would you mind taking a look at this pull request? It's been a while since the last commit was reviewed.

ghost commented Mar 3, 2016

@spicyj would you mind taking a look at this pull request? It's been a while since the last commit was reviewed.

@mkonicek

This comment has been minimized.

Show comment
Hide comment
@mkonicek

mkonicek Mar 20, 2016

Contributor

@josebalius Can you please indicate whether this PR is still needed? See the comment by Brent Vatne:

if #113 works (assuming we can resolve the issue you mentioned in #113 (comment)) is this still needed?

Contributor

mkonicek commented Mar 20, 2016

@josebalius Can you please indicate whether this PR is still needed? See the comment by Brent Vatne:

if #113 works (assuming we can resolve the issue you mentioned in #113 (comment)) is this still needed?

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost May 20, 2016

It's been a while since the last commit was reviewed and the labels show this pull request needs review. Based on the blame information for the files in this pull request we identified @nicklockwood as a potential reviewer. Could you take a look please or cc someone with more context?

ghost commented May 20, 2016

It's been a while since the last commit was reviewed and the labels show this pull request needs review. Based on the blame information for the files in this pull request we identified @nicklockwood as a potential reviewer. Could you take a look please or cc someone with more context?

@josebalius

This comment has been minimized.

Show comment
Hide comment
@josebalius

josebalius May 20, 2016

I think this can probably be closed guys.

I think this can probably be closed guys.

@mkonicek

This comment has been minimized.

Show comment
Hide comment
@mkonicek

mkonicek May 25, 2016

Contributor

Thanks for looking @josebalius. I assume this was resolved in #113?

We are exposing TextInputState in #3308 which will allow you to do this too. I'm not sure it's worth exposing this one line function as it will increase API surface area ...

Contributor

mkonicek commented May 25, 2016

Thanks for looking @josebalius. I assume this was resolved in #113?

We are exposing TextInputState in #3308 which will allow you to do this too. I'm not sure it's worth exposing this one line function as it will increase API surface area ...

@mkonicek mkonicek closed this May 25, 2016

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