Skip to content

Commit e5de2b5

Browse files
elements: fix inconsistent API, UI, and DX issues
# What This commit addresses a number of small DX problems that were getting in the way of a decent build-out. - Removed all the Element Interface definitions; they weren’t adding anything to the value of the component. The Builders all use the Class definitions anyway. - Made the builder Props consistent. Some had an `Ak` prefix, some didn’t. I’ve gone with “no Ak prefix” on types being imported for builders. - Added missing t-shirt sizes, even when they’re defaults. - Changed the names of some attributes to be more “positive” about what they mean: “no-loading-message” -\> “spinner-only”; “no-icon” -\> “text-only”; “no-arrow” -\> “hide-arrow”. - Removes `reflect` where it’s not needed. Where it is necessary to trigger CSS effects when the value is dynamically changed (however unlikely that may be), I have added comments to say why reflection is required. - Fix the issue in the Empty State builder where it was generating unneeded slotted divs for TemplateResults. - Removed the `attribute` setting from Tooltip.target; it’s an Object, and cannot have an `attribute` key.
1 parent 2d2225b commit e5de2b5

File tree

6 files changed

+50
-33
lines changed

6 files changed

+50
-33
lines changed

src/ak-brand/ak-brand.component.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@ export class Brand extends LitElement {
2626
static readonly styles = [styles];
2727

2828
/** @attr {string} src - The URL for the image source */
29-
@property({ type: String, reflect: true })
29+
@property({ type: String })
3030
src?: string;
3131

3232
/** @attr {string} alt - The alt text for the image (for accessibility) */
33-
@property({ type: String, reflect: true })
33+
@property({ type: String })
3434
alt?: string;
3535

3636
render() {

src/ak-button/ak-button.component.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,9 @@ export class Button extends LitElement {
109109

110110
static readonly formAssociated = true;
111111

112+
// While it's unlikely that client code will modify these by manipulating `type` and `variant`
113+
// fields directly, it's still possible. Their presence triggers corresponding CSS effects, so
114+
// they must be monitored and reflected.
112115
@property({ reflect: true })
113116
public type?: ButtonType = "button";
114117

src/ak-empty-state/ak-empty-state.builder.ts

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,15 @@ export interface EmptyStateProps extends Partial<
1919
secondaryActions?: string | TemplateResult;
2020
}
2121

22+
const SLOTNAMES: (keyof EmptyStateProps)[] = [
23+
"icon",
24+
"title",
25+
"body",
26+
"footer",
27+
"actions",
28+
"secondaryActions",
29+
];
30+
2231
/**
2332
* @summary Helper function to create an EmptyState component programmatically
2433
*
@@ -27,19 +36,25 @@ export interface EmptyStateProps extends Partial<
2736
* @see {@link EmptyState} - The underlying web component
2837
*/
2938
export function akEmptyState(options: EmptyStateProps) {
30-
const {
31-
size,
32-
fullHeight,
33-
spinnerOnly,
34-
textOnly,
35-
loading,
36-
icon,
37-
title,
38-
body,
39-
footer,
40-
actions,
41-
secondaryActions,
42-
} = options;
39+
const slots = SLOTNAMES.filter((s) => !!options[s]);
40+
41+
let opts = {
42+
...options,
43+
...Object.fromEntries(
44+
slots.map((s) => [
45+
s,
46+
typeof options[s] === "string"
47+
? html`<div slot=${s === "secondaryActions" ? "secondary-actions" : s}>
48+
${options[s]}
49+
</div>`
50+
: options[s],
51+
]),
52+
),
53+
};
54+
console.log(opts);
55+
56+
const { size, fullHeight, spinnerOnly, textOnly, loading } = opts;
57+
4358
return html`
4459
<ak-empty-state
4560
?loading=${loading}
@@ -48,12 +63,7 @@ export function akEmptyState(options: EmptyStateProps) {
4863
?spinner-only=${spinnerOnly}
4964
size=${ifDefined(String(size))}
5065
>
51-
${icon ? html`<div slot="icon">${icon}</div>` : ""}
52-
${title ? html`<div slot="title">${title}</div>` : ""}
53-
${body ? html`<div slot="body">${body}</div>` : ""}
54-
${footer ? html`<div slot="footer">${footer}</div>` : ""}
55-
${actions ? html`<div slot="actions">${actions}</div>` : ""}
56-
${secondaryActions ? html`<div slot="secondary-actions">${secondaryActions}</div>` : ""}
66+
${slots.map((s) => opts[s])}
5767
</ak-empty-state>
5868
`;
5969
}

src/ak-empty-state/ak-empty-state.component.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,14 @@ export class EmptyState extends LitElement {
6262
@property({ type: Boolean, attribute: "text-only" })
6363
public textOnly = false;
6464

65-
@property({ type: Boolean, reflect: true, attribute: "spinner-only" })
65+
@property({ type: Boolean, attribute: "spinner-only" })
6666
public spinnerOnly = false;
6767

6868
@property({ type: Boolean })
6969
public loading = false;
7070

71-
@property({ type: String })
71+
// A case where external manipulation of must be reflected to trigger CSS effects.
72+
@property({ type: String, reflect: true })
7273
public size: EmptyStateSize = "lg";
7374

7475
#onSlotChange = () => {

src/ak-empty-state/ak-empty-state.stories.ts

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -309,25 +309,28 @@ export const HelperFunction: Story = {
309309
render: () => html`
310310
<div style="display: flex; flex-direction: column; gap: 2rem;">
311311
${akEmptyState({
312-
icon: html`<ak-icon icon="fa fa-beer"></ak-icon>`,
312+
icon: html`<ak-icon slot="icon" icon="fa fa-beer"></ak-icon>`,
313313
title: "Hold My Beer",
314314
body: "I saw this in a cartoon once. I'm sure I can pull it off.",
315-
actions: html`<button>Leave The Scene Immediately</button>`,
315+
actions: html`<button slot="actions">Leave The Scene Immediately</button>`,
316316
})}
317317
${akEmptyState({
318318
size: "sm",
319319
textOnly: true,
320-
title: html`<h3>Without Icon</h3>`,
321-
body: html`<p>Created using the helper function with textOnly=true</p>`,
322-
actions: html`<button>Action Button</button>`,
320+
title: html`<h3 slot="title">Without Icon</h3>`,
321+
body: html`<p slot="body">Created using the helper function with textOnly=true</p>`,
322+
actions: html`<button slot="actions">Action Button</button>`,
323323
})}
324324
${akEmptyState({
325325
size: "lg",
326326
fullHeight: false,
327-
title: html`<h2>Default Icon</h2>`,
328-
body: html`<p>Using the default icon since none provided and textOnly=false</p>`,
329-
actions: html`<button>Primary Action</button><button>Secondary Action</button>`,
330-
footer: html`<a href="#">Learn more about this state</a>`,
327+
title: html`<h2 slot="title">Default Icon</h2>`,
328+
body: html`<p slot="body">
329+
Using the default icon since none provided and textOnly=false
330+
</p>`,
331+
actions: html`<button slot="actions">Primary Action</button
332+
><button>Secondary Action</button>`,
333+
footer: html`<a href="#" slot="footer">Learn more about this state</a>`,
331334
})}
332335
</div>
333336
`,

src/ak-tooltip/ak-tooltip.component.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ export class Tooltip extends LitElement {
144144
* @attr {object} target: A reference to the target. Must be in the same or in a sibling context
145145
of the tooltip. `.target` takes precedence over `for`
146146
*/
147-
@property({ type: Object, attribute: "target" })
147+
@property({ type: Object })
148148
public target?: HTMLElement;
149149

150150
/**

0 commit comments

Comments
 (0)