-
Notifications
You must be signed in to change notification settings - Fork 55
fix(Input): changing the default styles for Input component #25
Conversation
Codecov Report
@@ Coverage Diff @@
## master #25 +/- ##
==========================================
- Coverage 87.37% 87.12% -0.25%
==========================================
Files 74 74
Lines 1125 1142 +17
Branches 200 202 +2
==========================================
+ Hits 983 995 +12
- Misses 138 143 +5
Partials 4 4
Continue to review full report at Codecov.
|
src/components/Input/inputRules.ts
Outdated
@@ -9,13 +9,22 @@ const inputRules = { | |||
borderRadius: variables.borderRadius, | |||
outline: 0, | |||
padding: variables.defaultPadding, | |||
backgroundColor: variables.backgroundColor, | |||
':focus-within': { |
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.
:focus-within
isn't supported in IE/Edge
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.
@codepretty, thanks for the help. I changed the approach to move input at the full size of the parent div and basically the focus will stay attached to the input, in a classic way that works in Edge
When I click on the search icon, the input doesn't get focused. |
fc18596
to
60739ac
Compare
src/components/Input/Input.tsx
Outdated
renderComponent({ ElementType, classes, rest }) { | ||
const { children, className, icon, input, type } = this.props | ||
const [htmlInputProps, restProps] = this.partitionProps() | ||
|
||
const inputClasses = cx(classes.input) | ||
const iconClasses = cx(classes.icon) |
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.
No need for using cx here, as the classes are already calculated and we are not combining multiple object to classes.
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.
fixed
src/components/Input/Input.tsx
Outdated
@@ -108,9 +126,15 @@ class Input extends UIComponent<any, any> { | |||
<ElementType {...rest} className={classes.root} {...htmlInputProps}> | |||
{createHTMLInput(input || type, { | |||
defaultProps: htmlInputProps, |
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.
We can extract the creation of the input in a separate variable and reuse it in the both return statements. I would actually refactor this with the following:
return (<ElementType {...rest} className={classes.root} {...htmlInputProps}>
{createHTMLInput(input || type, {
defaultProps: htmlInputProps,
overrideProps: {
className: inputClasses,
ref: this.handleInputRef,
},
})}
{this.computeIcon() && Icon.create(this.computeIcon(), {
defaultProps: { className: iconClasses },
overrideProps: predefinedProps => this.handleIconOverrides,
})}
</ElementType>)
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.
Thanks. Fixed.
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.
I left couple of comments, other than that it looks good to be merged here.
@codepretty , thanks. Fixed. |
Input
Updating the default classes for the input to match the light view of the Teams UI.
TODO
UI Update
Simple input
Simple input focused
Input variation with icon