feat(Label): add LabelGroup and missing docs #576
Conversation
Current coverage is 99.52% (diff: 100%)@@ master #576 diff @@
==========================================
Files 118 119 +1
Lines 1870 1878 +8
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 1863 1869 +6
- Misses 7 9 +2
Partials 0 0
|
|
@levithomason @jcarbo I need review there |
| import React from 'react' | ||
| import { Grid, Image, Label, Segment } from 'stardust' | ||
|
|
||
| const imageStyles = { |
There was a problem hiding this comment.
Are these style required? I'd like to avoid any styles in the docs. It would seem that SUI alone should handle all styling.
There was a problem hiding this comment.
If I remove them, example will look ugly 😿
There was a problem hiding this comment.
I don't think we should ever include styles in the examples. I think it gives the impression that we require inline styles to be used for the example to work. If we truly must include styles, there are doc site css rules in the index.ejs template for this, but let's not put them in the JSX.
In this case, I think there is little enough difference that we can just leave them out:
Alternatively, we could use <Segment padded> to give whitespace similar to what the inline image styles did:
| import React from 'react' | ||
| import { Grid, Image, Label, Segment } from 'stardust' | ||
|
|
||
| const imageStyles = { |
There was a problem hiding this comment.
Same regarding styles here, any way to remove them?
| import { Label } from 'stardust' | ||
|
|
||
| const LabelExampleCircular = () => { | ||
| const colors = [ |
There was a problem hiding this comment.
When we have data for examples, let's put it up top outside the scope of the example itself.
There was a problem hiding this comment.
Same comment re: sizes, should use Label._meta.props.color
| import { Label } from 'stardust' | ||
|
|
||
| const LabelExampleSize = () => { | ||
| const sizes = ['mini', 'tiny', 'small', 'medium', 'large', 'big', 'huge', 'massive'] |
There was a problem hiding this comment.
More data here, let's move it up top. There are several other examples with data to update so I won't comment on all of them.
There was a problem hiding this comment.
Actually, I think better would be to just use Label._meta.props.size, this way the sizes we give examples for are all the sizes the component supports. There are a few other places in the docs we should update to do this as well.
There was a problem hiding this comment.
I don't want to expose the private _meta property. It may change or completely go away in the future.
There was a problem hiding this comment.
More info, meta used to have more keys and have more purpose. Over time, it keeps shrinking. Considering the parent, type, and name keys are only for documentation, I hope to pull them out of the code and into the doc block. We already parse the docblock for docs so grabbing type, parent, and name from there makes sense.
This leaves only the props in the meta. In farther out, less defined goals, I'd like to better formalize how we define props to support the missing abilities of the component explorer #427.
|
Updated to |
levithomason
changed the title
|
Released in |
This PR:
Groupcomponent;Requires merge of #551.