Add EuiEmptyPrompt. #711
Add EuiEmptyPrompt. #711
Conversation
cjcenizal
force-pushed
the
empty-prompt
branch
3 times, most recently
from
14aefe1 to
3d1fce3
Compare
|
@cjcenizal In a separate PR I would likely add the ability for this to be paneled (and centered) and to pass a titleSize prop. Think those additions would give us 90% empty state use cases so far in design. |
|
@snide Just curious: what are the situations in which different title sizes will be used? If we want that kind of flexibility it might just make sense to remove the |
There was a problem hiding this comment.
I agree with @snide that we will want to add paneled/paged versions to this. I foresee an situation where we'd want to change the title size depending on the surrounding layout. If the object that is empty is a minor part of the layout, this heading may be too large.
| if (iconType) { | ||
| icon = ( | ||
| <Fragment> | ||
| <EuiIcon type={iconType} size="xxl" color="subdued" /> |
There was a problem hiding this comment.
We should allow the user to display the icon in any color.
|
|
||
| if (Array.isArray(actions)) { | ||
| actionsRow = ( | ||
| <EuiFlexGroup gutterSize="m" alignItems="center" justifyContent="center"> |
There was a problem hiding this comment.
Actions within a block of centered text should be stacked with the primary action on top, not horizontal. Similar to the way cards handles this:
src/components/text/_mixins.scss
Outdated
| max-width: 36em; | ||
| // If the max-width is way too short of the width of the container, | ||
| // at least make it 2/3 of it's parent | ||
| min-width: 75%; |
There was a problem hiding this comment.
I'm not a fan of having this min-width on centered content as long line lengths on centered content is very hard to read and since we're centered we don't have to worry about the space we take up compared to other parts of the layout.
Instead of making this a mixin, can you just change the 36em into a variable and use that?
| <Fragment> | ||
| <EuiText> | ||
| {body} | ||
| </EuiText> |
There was a problem hiding this comment.
Will this support callouts and other types of content that isn't just plain text?
There was a problem hiding this comment.
It doesn't. Should we make that a requirement?
There was a problem hiding this comment.
I'm honestly on the fence, I feel like the flexibility would be nice, but it would be very rare cases... @snide?
There was a problem hiding this comment.
I think it's OK as is. For rare cases we can always build something unique when we need it.
|
@cjcenizal Think @cchaos covered it, but the two common scenarios we often design are
Your table example is the later, and the sizing should be smaller. |
|
@cchaos @snide Thanks for the feedback! I've addressed everything.
The user can now set the title size and the button size, though I think the size of the icon and the vertical spacing becomes a little disproportionate as these sizes change. It might be better to provide a single "size" prop, which then scales the size of the icon, title, text, the buttons, and the vertical spacers between everything internally. Per Dave's examples, the first example could be the default size, and the second one could be a "s" size. I'd just need to know what specifications you want for everything, or we could just remove the |
| }; | ||
|
|
||
| EuiEmptyPrompt.propTypes = { | ||
| iconType: PropTypes.string, |
There was a problem hiding this comment.
Should the iconTypes be imported from EuiIcon?
| You can remove parts of the prompt to simplify it, if you wish. | ||
| </p> | ||
| <p> | ||
| You can also provide an array of multiple actions. |
There was a problem hiding this comment.
Can you add a sentence here: "Be sure to list primary actions first and secondary actions last."
|
Good catches, @cchaos. Addressed. |
|
jenkins test this |
Also supports less content and more actions:
Works well in tables: