Empty state component #476
Conversation
| {{else if @image}} | ||
| <OSS::Badge @icon={{@image}} @size="lg" /> |
There was a problem hiding this comment.
@badgeIcon instead of @image would be better imo
| @@ -0,0 +1,22 @@ | |||
| <div class="fx-1 fx-col fx-xalign-center fx-gap-px-18"> | |||
| {{#if (has-block "image")}} | |||
| <span class=""> | |||
There was a problem hiding this comment.
why the span here? as it's a named block, let's just let the caller handle the content here. same below for the actions
| <div class="font-color-gray-900 font-size-{{this.titleSize}} font-weight-semibold"> | ||
| {{@title}} | ||
| </div> | ||
| <div class="font-color-gray-500 font-size-{{this.subtitleSize}}"> | ||
| {{@subtitle}} | ||
| </div> |
There was a problem hiding this comment.
title & subtitle are marked as optional in the TS but there is no check on their presence here, leading to empty divs.
I think we can remove the optional side in the TS and make both mandatory for now (i don't remember seeing an empty state w/ only a subtitle, or only a title)
Also, i think the div can be span instead.
| get subtitleSize(): string { | ||
| return this.args.size || 'md'; | ||
| } | ||
| } |
There was a problem hiding this comment.
you could add a size getter that returns this.args.size ?? 'md' and use it locally. or even:
get size(): string {
return ['sm', 'md'].includes(this.args.size) ? this.args.size : 'md';
}
(so we properly handle invalid sizes)
| @@ -0,0 +1,18 @@ | |||
| <div class="fx-1 fx-col fx-xalign-center fx-gap-px-18"> | |||
There was a problem hiding this comment.
I think we should include splattributes here just to be safe
Aside from the classes, it will allow us to define a data-control-name in the parent if needed
| return this.size === 'sm' ? 'md' : 'lg'; | ||
| } | ||
|
|
||
| get size(): string { |
There was a problem hiding this comment.
nit: maybe have subtitleSize here to match naming scheme with titleSize ?
There was a problem hiding this comment.
I have mixed feelings about this because it's also used for invalid values handling
Or should I add a subtitleSize getter that just pass the size getter ?
| control: { type: 'text' } | ||
| }, | ||
| title: { | ||
| description: 'The title of the state', |
There was a problem hiding this comment.
Nit: I would remove "state" alone because it can have loads of definitions which are not in this context.
How about
title: "A title displayed below the icon or badge in the component" ?
subtitle: "A subtitle displayed under the title in the component"
| assert.dom('.upf-badge').exists(); | ||
| }); | ||
|
|
||
| test('it supports block usage for image', async function (assert) { |
There was a problem hiding this comment.
nit: named-block instead of just block, same at line 39
| assert.dom('button').hasText('Retry'); | ||
| }); | ||
|
|
||
| test('it applies size-based styles', async function (assert) { |
upfluence-bot
added
reviewed/needs-work
and removed
reviewed/needs-review
reviewed/needs-work
labels
| } | ||
|
|
||
| get size(): string { | ||
| return this.args.size && ['sm', 'md'].includes(this.args.size) ? this.args.size : 'md'; |
There was a problem hiding this comment.
You can set ['sm', 'md'] in const with ALLOWED_SIZE for example of wording
| assert.dom('button').hasText('Retry'); | ||
| }); | ||
|
|
||
| test('it applies size-based styles', async function (assert) { |
There was a problem hiding this comment.
Missing not allowed size like random
| size?: 'sm' | 'md'; | ||
| } | ||
|
|
||
| const ALLOWED_SIZE = ['sm', 'md']; |
There was a problem hiding this comment.
| const ALLOWED_SIZE = ['sm', 'md']; | |
| const ALLOWED_SIZES = ['sm', 'md']; |
My bad I forget the "s" at the end, don't forget the type also
upfluence-bot
added
reviewed/needs-review
reviewed/needs-work
and removed
reviewed/needs-review
labels
| parameters: { | ||
| docs: { | ||
| description: { | ||
| component: 'An component used when there is nothing to display on a page' |
There was a problem hiding this comment.
Small typo here, should be A component instead of an :)
What does this PR do?
This PR adds an empty state component
What are the observable changes?
Good PR checklist