feat: oui skeleton #185
feat: oui skeleton #185
Conversation
|
|
||
| export default { | ||
| controller, | ||
| controllerAs: "$ctrl", |
There was a problem hiding this comment.
this is the default value, you can remove it
| <span class="oui-skeleton__loader" | ||
| ng-show="$ctrl.loading" | ||
| data-ng-style="{width: $ctrl.width}"></span> | ||
| <div ng-hide="$ctrl.loading" ng-transclude></div> |
packages/oui-skeleton/README.md
Outdated
| @@ -0,0 +1,28 @@ | |||
| # Skeleton | |||
|
|
|||
| <component-status cx-design="complete" ux="complete"></component-status> | |||
There was a problem hiding this comment.
Update ux="complete" to rc. complete are only for components already integrated and tested in our apps.
packages/oui-skeleton/README.md
Outdated
|
|
||
| | Attribute | Type | Binding | One-time Binding | Values | Default | Description | ||
| | ---- | ---- | ---- | ---- | ---- | ---- | ---- | ||
| | width | string | @ | | | random (<100 or >30) | width of the element in percentage ("30%") |
There was a problem hiding this comment.
I think we should go with an attribute size, like we have here: https://master.ui-kit.ovh/#!/ovh-ui-kit/field, https://master.ui-kit.ovh/#!/oui-angular/field (or the spinner https://master.ui-kit.ovh/#!/oui-angular/spinner).
packages/oui-skeleton/README.md
Outdated
| | Attribute | Type | Binding | One-time Binding | Values | Default | Description | ||
| | ---- | ---- | ---- | ---- | ---- | ---- | ---- | ||
| | width | string | @ | | | random (<100 or >30) | width of the element in percentage ("30%") | ||
| | loading | boolean | < | | true, false | | boolean to indicate to show or hide the component |
There was a problem hiding this comment.
Useless, could be done with an ngIf :)
See my comment about transclude first.
| "ngInject"; | ||
|
|
||
| this.$attrs = $attrs; | ||
| this.max = 100; |
There was a problem hiding this comment.
This should be in CSS with max-width and min-width.
But like my upper comment said, we should go with sizes instead (s, m, l). By default, it could auto like we have for this component: https://master.ui-kit.ovh/#!/oui-angular/field
What do you think ? :)
There was a problem hiding this comment.
Yup this should be good. Better than choosing random width. Will do this
| loading: "<", | ||
| width: "@" | ||
| }, | ||
| transclude: true |
There was a problem hiding this comment.
I think this component should be used more like the spinner, not like a container that shows the content or not.
To be discussed
There was a problem hiding this comment.
The transclude actually helps to show the skeleton before the actual component is loaded. When you have lots of fields in a page, transclude makes it easier to wrap the skeleton on every component and show the actual component when loaded.
Other than this, there is no other real use of it. But this also means showing whichever loads first or like showing multiple spinners. Which might not look so good.
AxelPeter
requested review from
AxelPeter,
JDownloader,
neolitec,
rfougier and
whispyy
|
@AxelPeter have a look at it now. I think this should do :) |
| <oui-skeleton></oui-skeleton> | ||
| ``` | ||
|
|
||
| ### Size |
There was a problem hiding this comment.
Maybe size is not the thing we want.
Typically the skeleton graphically represents a text we don't know in advance. So the size should be... randomized :)
Maybe the right parameters here should be "min-width" with a default value and eventually "width" / "auto" if we want to force the width... ?
In my opinion, "min-width" to 30% by default for example so that the skeleton always take between 30% and 100% of the width.
@AxelPeter what do you think about?
There was a problem hiding this comment.
Nice idea. But I don't think it's the case for all uses.
We can even have both with an attribute "randomized" for example :)
In fact, I'm not a fan of replacing style="width: 80%" just by width="80%". That's why by default, I let the skeleton with auto to let the user set its width.
I'll check to add your idea
add oui skeleton component add examples and test case cui skeleton to oui
add default parameter bind loading in readme cui skeleton to oui
remove controllerAs ng if used instead of ng show cui skeleton to oui
remove loading in skeleton remove width calculation add size attr to skeleton cui skeleton to oui
AxelPeter
force-pushed
the
feature/oui-skeleton
branch
from
1447a14 to
aa29fa4
Compare
5 participants
add oui skeleton component
add examples and test case
cui skeleton to oui
JIRA: UU-845