fix(Input): changing the default styles for Input component #25
Conversation
alinais
requested review from
dvdzkwsk,
kuzhelov,
levithomason and
mnajdova
as code owners
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.
|
|
Looks like the bottom focus border angle is inverted. It should be angled up at the corners, like this... |
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.
:focus-within isn't supported in IE/Edge
There was a problem hiding this comment.
@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. |
alinais
force-pushed
the
feature/input-component-base-theme-light-teams
branch
from
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.
No need for using cx here, as the classes are already calculated and we are not combining multiple object to classes.
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.
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>)
|
If you have time, we should update the doc to just be "an icon" instead of "a search icon" since it really could be anything there. |
|
@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
