-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[InputAction] Initial implementation of a InputAction
component
#8365
Conversation
function InputAction(props: Default & Props) { | ||
const { children, component, classes, className, ...other } = props; | ||
|
||
const Component = component || 'div'; |
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.
React provides a mechanism for default, let's use it, it automatically added in the docs.
*/ | ||
className?: string, | ||
/** | ||
* |
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.
Missing
I have been giving more thought about this issue. I think that I have found an elegant solution. Let me know what you think about it. It's some code I typed in my editor. It's not supposed to work, just to give an idea. <FormControl className={classes.formControl}>
<InputLabel htmlFor="name-helper">Name</InputLabel>
<InputAction
input={
<Input id="name-helper" value={this.state.name} onChange={this.handleChange} />
}
action={
<IconButton onClick={this.toggleShowPassword}>
{this.state.showPassword?<Hide />:<Eye />}
</IconButton>
}
/>
<FormHelperText>Some important helper text</FormHelperText>
</FormControl>
<FormControl className={classes.formControl}>
<InputLabel htmlFor="name-helper">Name</InputLabel>
<PasswordInput>
<Input id="name-helper" value={this.state.name} onChange={this.handleChange} />
</PasswordInput>
<FormHelperText>Some important helper text</FormHelperText>
</FormControl>
function InputAction(props) {
return {
<div styles={{
position: 'relative'
}}>
{props.input}
{React.cloneElement(props.action, {
styles: {
position: 'absolute',
},
})}
</div>
}
}
class PasswordInput extends React.Component {
state = {
show: false,
}
render() {
return (
<InputAction
input={this.props.children}
action={
<IconButton onClick={this.toggleShowPassword}>
{this.state.show? <Hide /> : <Eye />}
</IconButton>
}
/>
)
}
} |
Pros:
|
I think that it should have a ripple.
I don't think that it's the right behavior. Regarding fixing it. I would expect the FormControl to listen for focus events. The event bubbles up from the IconButton. We can stop the propagation. |
<InputAction
input={
<Input id="name-helper" value={this.state.name} onChange={this.handleChange} />
}
action={
<IconButton onClick={this.toggleShowPassword}>
{this.state.showPassword?<Hide />:<Eye />}
</IconButton>
}
/> I like the above and its nice to not have to have context used but it feels slightly more like the approach taken in v0.1 rather than v1. With v1 children seem have have been used more. i.e. <Input ...>
<InputAction>.....</InputAction>
</Input> |
This is what we have been doing with the Aside of the PasswordInput, I think that we can have a look at the bigger picture. |
So how about something like: <Input>
<InputAdornment position='before'>$</InputAdornment>
<InputAdornment position='after'>.00</InputAdornment>
<InputAdornment position='after'>
<IconButton>
<DeleteIcon />
</IconButton>
</InputAdornment>
</Input> |
Just adding my two cents: Clicking the button should not give focus to the input, it's really annoying if the input is already focused and it gets re-focused when the password visibility is toggled (the carret will be moved). This can easily be handled by cancelling the event, though. Apart from that, I really like @eyn's approach. 🌈 |
Had a quick attempt to put together InputAdornment. The initial approach feels OK although React.Children.toArray() and filter() feels a little fragile. Styling right now is hacky - switched Input to be a flex when it is adorned but I assume there is good reason it is an inline-block normally. Button feels a long way from the ink line but looking at the material spec it seems in line with the spec. InputLabel also doesn't play nicely - one option would be to not render left adornments when shrink is false but its not clear from the Material Spec what the behaviour should be here. Generating errors for Select component for some reason at the moment that needs fixing. Tests are broken |
Closing in favour of #8504 |
This needs more work but thought I'd share this to try and get some initial feedback/thoughts. I'm not totally confident on the theme styling and I'm also not sure the button should have ripple etc. Also clicking the button gives input focus which I'm not convinced is the right behaviour (It feels odd when value is empty so label animates when clicking on button). Not sure best way to stop that at the moment.
Any thoughts on approach/alternatives would be good.
Should make it easy to create composed components with actions such as PasswordInput (#6693)