Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

va-button: add loading variation #1446

Merged
merged 8 commits into from
Jan 9, 2025
Merged

va-button: add loading variation #1446

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
db8bc4b
va-button: add loading variation
powellkerry Jan 7, 2025
42b42b5
Merge branch 'main' into 3460-button-loading
powellkerry Jan 7, 2025
f190471
Merge branch 'main' into 3460-button-loading
powellkerry Jan 8, 2025
998d664
Update var definition to let
powellkerry Jan 8, 2025
ef7bff5
Merge branch 'main' into 3460-button-loading
powellkerry Jan 8, 2025
47be963
update description for loading prop
powellkerry Jan 8, 2025
99f0fe1
remove aria-live on button
powellkerry Jan 9, 2025
937adff
fix tests
powellkerry Jan 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions packages/storybook/stories/va-button-uswds.stories.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ const defaultArgs = {
'secondary': undefined,
'primary-alternate': undefined,
'submit': undefined,
'text': 'Default',
'message-aria-describedby': 'Optional description text for screen readers',
onClick: (e) => console.log(e)
};

const Template = ({
Expand All @@ -34,12 +34,14 @@ const Template = ({
_continue,
disableAnalytics,
disabled,
loading,
label,
secondary,
primaryAlternate,
submit,
text,
messageAriaDescribedby,
onClick
}) => {
return (
<va-button
Expand All @@ -48,12 +50,13 @@ const Template = ({
continue={_continue}
disable-analytics={disableAnalytics}
disabled={disabled}
loading={loading}
label={label}
secondary={secondary}
primary-alternate={primaryAlternate}
submit={submit}
text={text}
onClick={e => console.log(e)}
text={!loading && !text ? 'Default' : text}
onClick= {onClick}
message-aria-describedby={messageAriaDescribedby}
/>
);
Expand Down Expand Up @@ -103,8 +106,22 @@ Back.args = {
export const Disabled = Template.bind(null);
Disabled.args = {
...defaultArgs,
disabled: true,
text: "Disabled",
disabled: true
};

export const Loading = Template.bind(null);
Loading.args = {
...defaultArgs,
text: 'Click to load',
onClick: (e) => {
e.target.setAttribute('text', '');
e.target.setAttribute('loading', 'true');
setTimeout(() => {
e.target.setAttribute('text', 'Click to load');
e.target.setAttribute('loading', 'false');
}, 5000)
}

};


Expand Down
8 changes: 8 additions & 0 deletions packages/web-components/src/components.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,10 @@ export namespace Components {
* The aria-label of the component.
*/
"label"?: string;
/**
* If `true`, the button will appear disabled, a loading icon will show next to the text, and the click event will not fire.
*/
"loading"?: boolean;
/**
* An optional message that will be read by screen readers when the input is focused.
*/
Expand Down Expand Up @@ -3502,6 +3506,10 @@ declare namespace LocalJSX {
* The aria-label of the component.
*/
"label"?: string;
/**
* If `true`, the button will appear disabled, a loading icon will show next to the text, and the click event will not fire.
*/
"loading"?: boolean;
/**
* An optional message that will be read by screen readers when the input is focused.
*/
Expand Down
88 changes: 80 additions & 8 deletions packages/web-components/src/components/va-button/test/va-button.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ describe('va-button', () => {
expect(element).toEqualHtml(`
<va-button class="hydrated" text="Edit">
<mock:shadow-root>
<button class="usa-button" part="button" type="button">
<span class="loading-message" role="status"></span>
<button aria-live="off" class="usa-button" part="button" type="button">
Edit
</button>
</mock:shadow-root>
Expand Down Expand Up @@ -38,7 +39,8 @@ describe('va-button', () => {
expect(element).toEqualHtml(`
<va-button back="" class="hydrated">
<mock:shadow-root>
<button class="usa-button usa-button--outline" type="button" part="button">
<span class="loading-message" role="status"></span>
<button aria-live="off" class="usa-button usa-button--outline" type="button" part="button">
<va-icon class="hydrated"></va-icon>
Back
</button>
Expand All @@ -54,7 +56,8 @@ describe('va-button', () => {
expect(element).toEqualHtml(`
<va-button continue class="hydrated">
<mock:shadow-root>
<button class="usa-button" type="button" part="button">
<span class="loading-message" role="status"></span>
<button aria-live="off" class="usa-button" type="button" part="button">
Continue
<va-icon class="hydrated"></va-icon>
</button>
Expand All @@ -72,7 +75,8 @@ describe('va-button', () => {
expect(element).toEqualHtml(`
<va-button class="hydrated" label="Edit John Smith" text="Edit">
<mock:shadow-root>
<button aria-label="Edit John Smith" class="usa-button" part="button" type="button">
<span class="loading-message" role="status"></span>
<button aria-live="off" aria-label="Edit John Smith" class="usa-button" part="button" type="button">
Edit
</button>
</mock:shadow-root>
Expand All @@ -87,7 +91,8 @@ describe('va-button', () => {
expect(element).toEqualHtml(`
<va-button class="hydrated" text="Edit" submit>
<mock:shadow-root>
<button class="usa-button" type="submit" part="button">
<span class="loading-message" role="status"></span>
<button aria-live="off" class="usa-button" type="submit" part="button">
Edit
</button>
</mock:shadow-root>
Expand All @@ -102,14 +107,70 @@ describe('va-button', () => {
expect(element).toEqualHtml(`
<va-button class="hydrated" text="Edit" disabled>
<mock:shadow-root>
<button class="usa-button" aria-disabled="true" type="button" part="button">
<span class="loading-message" role="status"></span>
<button aria-live="off" class="usa-button" aria-disabled="true" type="button" part="button">
Edit
</button>
</mock:shadow-root>
</va-button>
`);
});

it('renders a loading button with default text', async () => {
const page = await newE2EPage();
await page.setContent('<va-button loading></va-button>');
const element = await page.find('va-button');
expect(element).toEqualHtml(`
<va-button class="hydrated" loading>
<mock:shadow-root>
<span class="loading-message" role="status">Loading</span>
<button aria-busy="true" aria-live="polite" class="usa-button" aria-disabled="true" type="button" part="button">
<va-icon aria-hidden="true" class="hydrated loading-icon"></va-icon>
Loading...
</button>
</mock:shadow-root>
</va-button>
`);
});

it('renders a loading button with prop text', async () => {
const page = await newE2EPage();
await page.setContent('<va-button loading text="Retrieving Data"></va-button>');
const element = await page.find('va-button');
expect(element).toEqualHtml(`
<va-button class="hydrated" loading text="Retrieving Data">
<mock:shadow-root>
<span class="loading-message" role="status">Loading</span>
<button aria-busy="true" aria-live="polite" class="usa-button" aria-disabled="true" type="button" part="button">
<va-icon aria-hidden="true" class="hydrated loading-icon"></va-icon>
Retrieving Data
</button>
</mock:shadow-root>
</va-button>
`);
});

it('Changes status text on loading prop change', async () => {
const page = await newE2EPage();
await page.setContent('<va-button loading text="Retrieving Data"></va-button>');
const element = await page.find('va-button');
expect(element).toEqualHtml(`
<va-button class="hydrated" loading text="Retrieving Data">
<mock:shadow-root>
<span class="loading-message" role="status">Loading</span>
<button aria-busy="true" aria-live="polite" class="usa-button" aria-disabled="true" type="button" part="button">
<va-icon aria-hidden="true" class="hydrated loading-icon"></va-icon>
Retrieving Data
</button>
</mock:shadow-root>
</va-button>
`);
element.setAttribute('loading', 'false');
await page.waitForChanges();
const loadingMessageEl = await element.shadowRoot.querySelector('span.loading-message');
expect(loadingMessageEl.innerHTML).toEqual('Loading complete');
});

it('ignores text value and displays Continue when continue is true', async () => {
const page = await newE2EPage();
await page.setContent('<va-button text="Edit" continue></va-button>');
Expand All @@ -131,7 +192,8 @@ describe('va-button', () => {
expect(element).toEqualHtml(`
<va-button class="hydrated" back continue>
<mock:shadow-root>
<button class="usa-button usa-button--outline" type="button" part="button">
<span class="loading-message" role="status"></span>
<button aria-live="off" class="usa-button usa-button--outline" type="button" part="button">
Continue
</button>
</mock:shadow-root>
Expand Down Expand Up @@ -181,6 +243,15 @@ describe('va-button', () => {
expect(clickSpy).toHaveReceivedEventTimes(0);
});

it(`doesn't fire click event when loading is true`, async () => {
const page = await newE2EPage();
await page.setContent('<va-button text="Edit" loading></va-button>');
const clickSpy = await page.spyOnEvent('click');
const button = await page.find('va-button >>> button');
await button.click();
expect(clickSpy).toHaveReceivedEventTimes(0);
});

it('has the correct aria-label when label is given', async () => {
const page = await newE2EPage();
await page.setContent(
Expand Down Expand Up @@ -212,7 +283,8 @@ it(`renders a default submit button variant`, async () => {
expect(element).toEqualHtml(`
<va-button class="hydrated" submit="" text="Submit">
<mock:shadow-root>
<button class="usa-button" type="submit" part="button">
<span class="loading-message" role="status"></span>
<button aria-live="off" class="usa-button" type="submit" part="button">
Submit
</button>
</mock:shadow-root>
Expand Down
30 changes: 30 additions & 0 deletions packages/web-components/src/components/va-button/va-button.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,37 @@

:host {
display: inline-block;

.loading-message {
opacity: 0.00001;
position: absolute;
pointer-events: none;
}

.loading-icon {
animation: spin 1.5s linear infinite;
}

.loading-icon.chromatic:after {
animation: none;
}

@media (prefers-reduced-motion) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a cool media feature. TIL!

.loading-icon {
animation: none;
}
}

@keyframes spin {
0% {
transform: rotate(0deg);
}
100% {
transform: rotate(360deg);
}
}
}

:host([disabled]:not([disabled='false'])) button {
pointer-events: none;
}
Expand Down
59 changes: 44 additions & 15 deletions packages/web-components/src/components/va-button/va-button.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
Listen,
Prop,
Element,
Watch,
} from '@stencil/core';
import classnames from 'classnames';

Expand All @@ -23,6 +24,8 @@ import classnames from 'classnames';
shadow: true,
})
export class VaButton {
private showCompletedMessage: boolean = false;

@Element() el: HTMLElement;

/**
Expand Down Expand Up @@ -50,6 +53,22 @@ export class VaButton {
*/
@Prop({ reflect: true }) disabled?: boolean = false;

/**
* If `true`, the button will appear disabled, a loading icon will show next to the text, and the click event will not fire.
*/
@Prop({ reflect: true }) loading?: boolean = false;

@Watch('loading')
announceLoadingChange(newValue: boolean, oldValue: boolean) {
if (oldValue && !newValue) {
let me = this;
this.showCompletedMessage = true;
setTimeout(() => {
me.showCompletedMessage = false;
}, 3000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand the need for a timeout and why this specific time is used. Do you mind clarifying?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The <span role="status"> will only be read by the screen reader if there are changes to the content of the span. This is the reasoning for the span always being rendered, even if the loading prop is false, because if we render it when the prop changes the reader will not count that as a change and "Loading" will never be read.

The same applies to this timeout. If there is not any time between when the completion message is set and when it is set to null, the reader won't detect the change and the message won't be read. Its an arbitrary number to guarantee the "Loading complete" is read.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. Thanks for the explanation!

}
}

/**
* The aria-label of the component.
*/
Expand Down Expand Up @@ -111,7 +130,7 @@ export class VaButton {
private getButtonText = (): string => {
if (this.continue) return 'Continue';
if (this.back) return 'Back';

if (this.loading && !this.text) return 'Loading...';
return this.text;
};

Expand Down Expand Up @@ -145,7 +164,7 @@ export class VaButton {
*/
@Listen('click')
handleClickOverride(e: MouseEvent) {
if (this.disabled) {
if (this.disabled || this.loading) {
e.preventDefault();
e.stopPropagation();
return;
Expand All @@ -159,6 +178,7 @@ export class VaButton {
back,
continue: _continue,
disabled,
loading,
getButtonText,
label,
submit,
Expand All @@ -171,7 +191,7 @@ export class VaButton {
const ariaDescribedbyIds =
`${messageAriaDescribedby ? 'button-description' : ''}`.trim() || null;

const ariaDisabled = disabled ? 'true' : undefined;
const ariaDisabled = disabled || loading ? 'true' : undefined;
const buttonText = getButtonText();

const type = submit !== undefined ? 'submit' : 'button';
Expand All @@ -185,18 +205,27 @@ export class VaButton {

return (
<Host>
<button
class={buttonClass}
aria-disabled={ariaDisabled}
aria-label={label}
aria-describedby={ariaDescribedbyIds}
type={type}
part="button"
>
{back && !_continue && <va-icon icon="navigate_far_before" />}
{buttonText}
{_continue && !back && <va-icon icon="navigate_far_next" />}
</button>
{/* This span must always be present for changes to be announced for the loading prop. It will not show visually or be read without content*/}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love the idea of this span being rendered for all buttons. I have tested it and it does not announce anything when there is not content, but it would be nice to figure out a way to only render it if the button will be used for loading. Hoping someone has an idea for how to do that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's a big deal to have a span always in the markup but was there some limitation with just doing conditional JSX? Something like:

{this.loading && (
    <span class="loading-message" role="status">....</span>
)}

<span class="loading-message" role="status">
{this.loading ? 'Loading' : this.showCompletedMessage ? 'Loading complete' : null}
</span>
<button
class={buttonClass}
aria-disabled={ariaDisabled}
aria-busy={loading ? 'true' : undefined}
aria-label={label}
aria-live={loading ? 'polite' : 'off'}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe we'll need this aria-live on the button itself, as the span will handle the live region. Leaving it in doesn't seem to cause any negative effects, but could lead to repetitive announcements. I tested this code without it and it announced as expected.

aria-describedby={ariaDescribedbyIds}
type={type}
part="button"
>
{back && !_continue && <va-icon icon="navigate_far_before" />}
{
loading ? <va-icon class="loading-icon" icon="autorenew" aria-hidden="true"/> : null
}
{buttonText}
{_continue && !back && <va-icon icon="navigate_far_next" />}
</button>
{messageAriaDescribedby && (
<span id="button-description" class="usa-sr-only">
{messageAriaDescribedby}
Expand Down
Loading