Skip to content
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

Remove setProps and replaceProps. #5570

Merged
merged 1 commit into from Dec 3, 2015

Conversation

Projects
None yet
6 participants
@jimfb
Copy link
Contributor

commented Nov 30, 2015

Remove setProps and replaceProps.

jim
@sophiebits

This comment has been minimized.

Copy link
Collaborator

commented Nov 30, 2015

👍

Can we get rid of TopLevelWrapper now? I don't remember.

@jimfb

This comment has been minimized.

Copy link
Contributor Author

commented Dec 1, 2015

I don't remember either, but we would probably need it for setstate/replacestate, right?

relevant diff: 643651b

@sophiebits

This comment has been minimized.

Copy link
Collaborator

commented Dec 1, 2015

That was so that we could enqueue updates to native components in the same way as composites – native components don't have setState so it might be irrelevant now.

@jimfb

This comment has been minimized.

Copy link
Contributor Author

commented Dec 3, 2015

Ok, removing TopLevelWrapper should be a separate diff anyway, so I'll merge this now and that will unblock us for that diff.

jimfb added a commit that referenced this pull request Dec 3, 2015

Merge pull request #5570 from jimfb/remove-setprops-replaceprops
Remove setProps and replaceProps.

@jimfb jimfb merged commit 50c7b17 into facebook:master Dec 3, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@sebmarkbage sebmarkbage referenced this pull request Jan 12, 2016

Closed

Umbrella 15.0 #4600

5 of 7 tasks complete
@trigun539

This comment has been minimized.

Copy link

commented Nov 29, 2016

Since getDomNode was removed. Is it possible to focus a list item?
Before:
this.ref.element.getDOMNode().focus();
Now?
this.element.focus(); // In my case, the element is a li not an input.
Tried both ways, and not working. Tried to find documentation about putting an element in focus, but only thing I saw was doing something like: this.input.focus();

Any suggestions? Doc link I could follow. Thanks

@gaearon

This comment has been minimized.

Copy link
Member

commented Nov 29, 2016

@trigun539

Is this a question about React or about DOM in general? In React, you no longer need getDOMNode() because you get the DOM node itself.

As to how focus() works, it might be better to consult DOM tutorials. It’s not specific to React, unless you can show there is something that works in DOM but breaks in React, in which case you would need to file a bug with a reproducing case.

I hope this helps!

@sophiebits

This comment has been minimized.

Copy link
Collaborator

commented Nov 29, 2016

@trigun539 You need to add a ref to the input element. See the docs for more details:

https://facebook.github.io/react/docs/refs-and-the-dom.html

@laumair

This comment has been minimized.

Copy link

commented Nov 29, 2016

@trigun539 @gaearon Have used something like this in one of my implementations for focusing an input element. Not sure if this is the best way but I am pretty sure this can help.

const InputField = (props) => {
  const { input, shouldFocusField } = props;
  const focusField = c => {
    if (c != null) {
      c.focus();
    }
  };

  return <input {...input} {...shouldFocusField ? { ref: focusField } : {}} />;
};

Where you can control the shouldFocusField boolean in the component where you use the InputField component. This would definitely work for any other component as well as far as I think. Hope this helps!

@trigun539

This comment has been minimized.

Copy link

commented Nov 29, 2016

Thanks for the help.

@trigun539

This comment has been minimized.

Copy link

commented Nov 29, 2016

I guess the real issue was related to this and not react: http://stackoverflow.com/questions/30213603/javascript-not-able-to-set-focus-to-first-li-element-within-ul. Thanks again. @gaearon @spicyj @laumair

Added the tab index and worked great!

@renovate renovate bot referenced this pull request Feb 2, 2018

Open

Update dependency react to v0.14.9 #29

0 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.