-
Notifications
You must be signed in to change notification settings - Fork 55
Conversation
Codecov Report
@@ Coverage Diff @@
## master #59 +/- ##
==========================================
+ Coverage 85.88% 86.94% +1.06%
==========================================
Files 74 75 +1
Lines 1098 1180 +82
Branches 215 216 +1
==========================================
+ Hits 943 1026 +83
+ Misses 149 148 -1
Partials 6 6 Continue to review full report at Codecov.
|
CHANGELOG.md
Outdated
@@ -26,7 +26,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm | |||
### Features | |||
- Update styles for Menu underlined primary @miroslavstastny ([#20](https://github.com/stardust-ui/react/pull/20)) | |||
- Add Avatar `getInitials` prop and `presenceIndicatorBackground` variable @mnajdova ([#38](https://github.com/stardust-ui/react/pull/38)) | |||
- Add 'fluid' variant and size variables to Image @kuzhelov ([#54](https://github.com/stardust-ui/react/pull/54)) | |||
- Add `fluid` variant and size variables to Image @kuzhelov ([#54](https://github.com/stardust-ui/react/pull/54)) |
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 :)
@@ -14,6 +14,8 @@ export default (siteVars: any) => { | |||
vars.inputPadding = `${pxToRem(6)} ${pxToRem(24)} ${pxToRem(6)} ${pxToRem(12)}` | |||
vars.inputFocusBorderColor = siteVars.brand | |||
vars.inputFocusBorderRadius = `${pxToRem(3)} ${pxToRem(3)} ${pxToRem(2)} ${pxToRem(2)}` | |||
vars.inputMaxWidth = '100%' |
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.
probably, at this point it is not necessary to introduce these customization mechanisms - at least maxWidth
seems to be an overkill with width
property being set to the same value. Also, if we would like to introduce width
variable, we should keep in mind that it is a more specific setting, so it should win fluent
prop setting for the following case:
<Input fluid /> { /* width is 100% */ }
<Input fluid variables={{ width: '50px' }} /> { /* width should be 50px, as 'variables' is more specific setting */ }
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.
fixing it by reducing the possibility to modify the width of the input tag from variables
src/components/Input/inputRules.ts
Outdated
return { | ||
display: 'inline-flex', | ||
position: 'relative', | ||
alignItems: 'center', | ||
justifyContent: 'flex-end', | ||
outline: 0, | ||
...(fluid && { | ||
width: '100%', | ||
maxWidth: '100%', |
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.
why we need to set maxWidth
in addition?
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.
removing it; not needed
e23bcad
to
2220834
Compare
Input
An input can have the full length of the parent element
TODO
API Proposal
Fluid
An input component can take the full width of the parent element.