Task/jenkins 42109 text input icons #148
Conversation
…s down props to the relevant text element
…re TextInput: splitting them up would probably lead to tedium long term as we add features like "clearButton" (the "X" on the right side that user can click to clear text)
|
cc @sophistifunk I actually went in a different direction from our last chat. Explanation: I suspect we're going to need a prop like So, if icons are only supported by a separate Now, maybe it's just a naming thing... and maybe I left this as WIP only because I wanted to get your feedback before I go any further. I think the only thing left though is to support our small / medium / large text sizes by scaling up the icon and tweaking its position. Will refactor that evil margin-left/right CSS into a mixin so it reads better. |
|
@sophistifunk perhaps you can review this one when you are online today? Thanks :) |
| } | ||
|
|
||
|
|
||
| .icon-rules(@icon-size, @icon-padding) { |
|
This is less code than I was expecting, nice wun bwuvva. 🐝 |
| return React.cloneElement( | ||
| child, | ||
| { | ||
| placeholder: this.props.placeholder, |
There was a problem hiding this comment.
Might be useful to pass this.props.inputProps or something along for all the other stuff that might be needed, e.g. onFocus etc.
| export class TextInput extends React.Component { | ||
|
|
||
| render() { | ||
| const classLeft = this.props.iconLeft ? 'u-icon-left' : ''; |
There was a problem hiding this comment.
what if you need multiple icons? (already had the column filter design needing that... well, really just needing extra elements)
There was a problem hiding this comment.
Can you point me to the design for that? You're saying there are multiple icons on the left or right?
| return ( | ||
| <TextControl {...this.props} className={`TextInput ${this.props.className}`}> | ||
| <TextControl {...this.props} className={`TextInput ${this.props.className} ${classLeft} ${classRight}`}> | ||
| { classLeft && <NestedIcon className={classLeft} icon={this.props.iconLeft} /> } |
There was a problem hiding this comment.
what happens if you want these icons to be clickable?
There was a problem hiding this comment.
I'll certainly add that in once we have a use case for it. Right now I'm supporting "icons as decoration" only.
|
@kzantow your comments here are valid, although I think they're all around future use cases and we can refine them as we go. Need to get this one through as it is blocking #151 and jenkinsci/blueocean-plugin#845. I will make sure this good feedback is taken into account when the time comes. |
Description
Submitter checklist
Reviewer checklist
@reviewbybees