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

[TextField] Adding the ability to call select on the TextField input #3287

Merged
merged 1 commit into from
Mar 12, 2016

Conversation

CoreyWinkelmannPP
Copy link

I would like a similar function like focus to exist for selecting the text inside the input on the TextField component. Here is a pull request adding that in.

@oliviertassinari
Copy link
Member

We try to remove as many imperative methods as possible.
What about using _getInputNode() for this use case and deprecating focus() and blur()?

@CoreyWinkelmannPP
Copy link
Author

I was using _getInputNode() but in my head I was thinking that the underscore had a meaning to it. Usually I see that and think this is not a public function and could be subject to change. If this is the desire going forward than I will keep to using that method.

I do think maybe having the function getInputNode() would be a good path forward and deprecating those other functions would make sense to me.

@oliviertassinari
Copy link
Member

I see that and think this is not a public function and could be subject to change.

You are right, but we will always try to be backward compatible with a deprecation notice before removing it.
Ideally, our unique API would be properties. However, the focus, blur and select are hard to get right.

@newoga
Copy link
Contributor

newoga commented Feb 11, 2016

My only fear with having the getInputNode() method is we're really coupling the fact a TextField component has an underlying <input /> control. Maybe there is no way around this though. I want to think about this some more.

@oliviertassinari
Copy link
Member

@callemall/material-ui I'm fine with this PR until we have a better option for focus() and blur().
We can easily deprecate it later if needed.

@alitaheri
Copy link
Member

@oliviertassinari We shouldn't deprecate these, these are valid use cases, we'll use warpgate later. and merging this won't introduce a breaking change later on. I'm going to merge this for now 👍

alitaheri added a commit that referenced this pull request Mar 12, 2016
[TextField] Adding the ability to call select on the TextField input
@alitaheri alitaheri merged commit 8087513 into mui:master Mar 12, 2016
@alitaheri
Copy link
Member

Oh shoot, we removed isMounted! I'll fix it rightaway.

alitaheri added a commit to alitaheri/material-ui that referenced this pull request Mar 12, 2016
This was introduced with mui#3287 as migration from
isMounted happened after this PR was opened.
aahan96 pushed a commit to aahan96/material-ui that referenced this pull request Mar 17, 2016
This was introduced with mui#3287 as migration from
isMounted happened after this PR was opened.
und3fined pushed a commit to und3fined/material-ui that referenced this pull request Mar 22, 2016
This was introduced with mui#3287 as migration from
isMounted happened after this PR was opened.
@zannager zannager added the component: text field This is the name of the generic UI component, not the React module! label Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: text field This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants