feat: new grid #25
feat: new grid #25
Conversation
it's ready to test now. |
src/layout/V2Grid/V2GridItem.js
Outdated
| * available for each row. Valid values are 1-12, "auto" or an object | ||
| * definining the size for each breakpoint individually like `{ xs: 12, sm: 6, md: 4, lg: 3 }`. | ||
| */ | ||
| size: oneOfType([number, string, shape({})]), |
There was a problem hiding this comment.
I would rather have explicit props xs={12} sm={6}.... This way we would also be able to replace the logic above by a simple array of class names similar to this
[
xs && olt.V2GridItemXs12,
...
]
Material UI and Bootstrap React do it this way
There was a problem hiding this comment.
I think the argument in the previous version was that breakpoints could be customized. But i guess that's a flawed argument because it's currently not really customizable. I will adjust this.
src/layout/V2Grid/V2GridItem.js
Outdated
| * available for each row. Valid values are 1-12 or an object | ||
| * definining the column offset for each breakpoint individually like `{ xs: 1, sm: 2, md: 3, lg: 4 }`. | ||
| */ | ||
| offset: oneOfType([number, shape({})]), |
There was a problem hiding this comment.
Here the same as the size prop
There was a problem hiding this comment.
should we call this offsetXs, offsetSm etc. ?
There was a problem hiding this comment.
and sizeXs, sizeXS or just xs?
There was a problem hiding this comment.
or better leave it as it is ;).
There was a problem hiding this comment.
@JonathanStoye what's your opinion here?
There was a problem hiding this comment.
I like offsetXs, offsetSm the most I guess. It is very expressive.
There was a problem hiding this comment.
@JonathanStoye @jkettmann ok, i will change that, but keep in mind that everyone who has to upgrade from v1 to v2 will hate us a bit here ;).
There was a problem hiding this comment.
I guess I will be one of those people :-)
4 participants
This PR is currently blocked by lightelligence-io/styles#19 and a subsequent npm release.