feat: improve the Spinner component #1586
Conversation
Features
Bug Fixes
Contributors |
src/components/Spinner/readme.md
Outdated
| @@ -115,3 +115,37 @@ const Loading = styled.h1.attrs(props => { | |||
| </Loading> | |||
| </InverseContainer> | |||
| ``` | |||
|
|
|||
| ##### list loading using spinner - brand - large - arc | |||
There was a problem hiding this comment.
I think this example names are missleading, there is nothing related to lists here. I am thinking about using something like Arc Spinner large brand; same for the rest of the exampe headers
src/components/Spinner/readme.md
Outdated
| </div> | ||
| ``` | ||
|
|
||
| ##### list loading using spinner - brand - x-large - arc - with logo |
|
There is a bug when spinner size is smaller than children, I believe we should limit the size of the icon to the inside of the circle. |
src/components/Spinner/index.js
Outdated
| @@ -6,23 +6,42 @@ import StyledArcSpinner from './styled/arcSpinner'; | |||
| import StyledSpinnerContainer from './styled/spinnerContainer'; | |||
| import StyledChildContainer from './styled/childContainer'; | |||
|
|
|||
| const sizeMap = { | |||
There was a problem hiding this comment.
since you are using the same size map in StyledArcSpinner i think you could put it in a separate file and import it from both places
…animation, modify comments in examples
|
+1 |
src/components/Spinner/index.js
Outdated
|
|
||
| if (isVisible) { | ||
| if (type === 'arc') { | ||
| return ( | ||
| <StyledSpinnerContainer> |
There was a problem hiding this comment.
we need to pass className and style prop to the container, all component containers receive these props in order to pass styles to positioning, margin and other styles they want to pass
…ah2107/react-rainbow into improve-the-Spinner-component
| if (size) { | ||
| return sizeMap[size] || sizeMap.medium; | ||
| } | ||
| return sizeMap.medium; |
There was a problem hiding this comment.
I think the if is too much with "return sizeMap [size] || sizeMap.medium;" the case that size is undefine is covered
fix: #1585
Changes proposed in this PR:
improve the Spinner component
I have followed (at least) the PR section of the contributing guide.
@nexxtway/react-rainbow