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.

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

fix(button): adds pending button, fixes #3162 #3163

Merged
merged 36 commits into from
Jan 5, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
305c7e6
feat(button): adds pending button, fixes #3162
benjamind Apr 28, 2023
52417fe
chore(button): fixed merge conflicts
TarunAdobe Dec 4, 2023
470b0f5
chore(button): import sp-progress-circle whenever required
TarunAdobe Dec 4, 2023
56e822c
chore: updated golden image hash
TarunAdobe Dec 4, 2023
dfc98fe
chore(button): updated aria-label on changing pending state
TarunAdobe Dec 4, 2023
0be1ae7
chore(button): removed redudant styles and functions
TarunAdobe Dec 5, 2023
b31e47a
chore(button): added tshirt sizing to the progress-circle
TarunAdobe Dec 7, 2023
49d9a8d
Merge branch 'main' into delarre/pending-button
TarunAdobe Dec 7, 2023
c34de46
Merge branch 'main' into delarre/pending-button
TarunAdobe Dec 11, 2023
312b676
chore(button): removed isPending state
TarunAdobe Dec 12, 2023
9353e4f
Merge branch 'main' into delarre/pending-button
TarunAdobe Dec 12, 2023
e305d44
chore(button): update aria-label on pending toggle
TarunAdobe Dec 12, 2023
fe59c9b
chore: fixed merge conflict
TarunAdobe Dec 13, 2023
f327507
chore: updated golden image hash
TarunAdobe Dec 13, 2023
3233d75
chore(button): removed ispending and timeout
TarunAdobe Dec 15, 2023
6acf088
chore: merged main and fixed conflict
TarunAdobe Dec 15, 2023
80f4baf
chore(button): updated css animation names
TarunAdobe Dec 18, 2023
e1650cb
Merge branch 'main' into delarre/pending-button
TarunAdobe Dec 18, 2023
1235028
chore(button): update aria-label on removing pending
TarunAdobe Dec 18, 2023
c1b54b5
chore: fixed merge conflicts
TarunAdobe Dec 19, 2023
3201c55
chore(button): added docs for pending state
TarunAdobe Dec 19, 2023
3c6eaeb
chore: updated golden image hash
TarunAdobe Dec 19, 2023
cdad70d
chore(button): update arialabel on disabled change in pending state
TarunAdobe Dec 20, 2023
4b3f03f
Merge branch 'main' into delarre/pending-button
TarunAdobe Dec 20, 2023
a96ae36
chore: updated golden image hash
TarunAdobe Dec 21, 2023
f67a9a5
fix(button): updated aria-label management in button
TarunAdobe Jan 3, 2024
db88621
fix(button): fixed aria-label management and added more tests
TarunAdobe Jan 3, 2024
740fb8c
chore: merged main
TarunAdobe Jan 3, 2024
b5e3523
fix(button): revert back the aria-label management logic in ButtonBase
TarunAdobe Jan 3, 2024
07e63b9
chore: updated golden image hash
TarunAdobe Jan 3, 2024
0258c3a
fix(button): shifted aria-label management to updated in button
TarunAdobe Jan 3, 2024
4c99a45
chore: merged main
TarunAdobe Jan 4, 2024
e8720ac
chore: udpated golden image hash
TarunAdobe Jan 4, 2024
d65480f
chore: updated golden image hash
TarunAdobe Jan 4, 2024
f36e8ad
chore(button): improved code readability
TarunAdobe Jan 5, 2024
3ffb308
Merge branch 'main' into delarre/pending-button
TarunAdobe Jan 5, 2024
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
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ executors:
parameters:
current_golden_images_hash:
type: string
default: d8be48bd925210181c3ec6933abbc3aade090bcb
default: 470b0f5294f4bffae31534b5734ecfd6317316ec
wireit_cache_name:
type: string
default: wireit
Expand Down
63 changes: 62 additions & 1 deletion packages/button/src/Button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,15 @@ governing permissions and limitations under the License.

import {
CSSResultArray,
html,
PropertyValues,
SizedMixin,
TemplateResult,
} from '@spectrum-web-components/base';
import { property } from '@spectrum-web-components/base/src/decorators.js';
import { ButtonBase } from './ButtonBase.js';
import buttonStyles from './button.css.js';
import { when } from '@spectrum-web-components/base/src/directives.js';

export type DeprecatedButtonVariants = 'cta' | 'overBackground';
export type ButtonStatics = 'white' | 'black';
Expand Down Expand Up @@ -51,6 +54,27 @@ export class Button extends SizedMixin(ButtonBase, { noDefaultSize: true }) {
return [...super.styles, buttonStyles];
}

protected pendingCooldown = -1;

// isPending represents the state of the button being in a pending state
// This also helps us show the progress circle and update the aria-label when the button is in pending state
@property({ type: Boolean, reflect: true, attribute: 'is-pending' })
public isPending = false;

@property({ type: String, attribute: 'pending-label' })
public pendingLabel = 'Pending';

// pending is the property that consumers can use to set the button into a pending state
@property({ type: Boolean, reflect: true, attribute: true })
public pending = false;

public override click(): void {
if (this.isPending) {
return;
}
super.click();
}

/**
* The visual variant to apply to this button.
*/
Expand Down Expand Up @@ -127,11 +151,30 @@ export class Button extends SizedMixin(ButtonBase, { noDefaultSize: true }) {
public set quiet(quiet: boolean) {
this.treatment = quiet ? 'outline' : 'fill';
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add back this line.

Copy link
Contributor

Choose a reason for hiding this comment

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

done hehe

public get quiet(): boolean {
return this.treatment === 'outline';
}

protected override willUpdate(
changedProperties: PropertyValues<this>
): void {
super.willUpdate(changedProperties);
if (changedProperties.has('pending')) {
if (this.pending) {
this.pendingCooldown = window.setTimeout(() => {
if (!this.disabled) {
this.isPending = true;
this.setAttribute('aria-label', this.pendingLabel);
}
}, 1000);
} else if (this.pending === false && this.isPending) {
window.clearTimeout(this.pendingCooldown);
this.isPending = false;
this.setAttribute('aria-label', this.label || '');
}
}
}

protected override firstUpdated(changes: PropertyValues<this>): void {
super.firstUpdated(changes);
// There is no Spectrum design context for an `<sp-button>` without a variant
Expand All @@ -140,4 +183,22 @@ export class Button extends SizedMixin(ButtonBase, { noDefaultSize: true }) {
this.setAttribute('variant', this.variant);
}
}

protected override renderButton(): TemplateResult {
return html`
${this.buttonContent}
${when(this.isPending, () => {
import(
'@spectrum-web-components/progress-circle/sp-progress-circle.js'
);
return html`
<sp-progress-circle
indeterminate
static="white"
label=${this.pendingLabel + 'progress circle'}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need the progress circle part? That forces un-translated content into the label.

Separately, is it even possible to get to the Progress Circle in the accessibility tree? Would it be better to use aria-hidden="true", as this is namely a visible affordance for the change in label on the Button?

Copy link
Contributor

Choose a reason for hiding this comment

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

you're right! this is just a visible affordance so it makes much more sense to have aria-hidden="true"

></sp-progress-circle>
`;
})}
`;
}
}
4 changes: 4 additions & 0 deletions packages/button/src/button-base.css
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ slot[name='icon']::slotted(img) {
--spectrum-ui-icon-tshirt-size-width: var(
--spectrum-alias-ui-icon-cornertriangle-size-75
);
--mod-progress-circle-size: var(--spectrum-alias-workflow-icon-size-s);
}

:host([size='m']) {
Expand All @@ -90,6 +91,7 @@ slot[name='icon']::slotted(img) {
--spectrum-ui-icon-tshirt-size-width: var(
--spectrum-alias-ui-icon-cornertriangle-size-100
);
--mod-progress-circle-size: var(--spectrum-alias-workflow-icon-size-m);
}

:host([size='l']) {
Expand All @@ -105,6 +107,7 @@ slot[name='icon']::slotted(img) {
--spectrum-ui-icon-tshirt-size-width: var(
--spectrum-alias-ui-icon-cornertriangle-size-200
);
--mod-progress-circle-size: var(--spectrum-alias-workflow-icon-size-l);
}

:host([size='xl']) {
Expand All @@ -120,4 +123,5 @@ slot[name='icon']::slotted(img) {
--spectrum-ui-icon-tshirt-size-width: var(
--spectrum-alias-ui-icon-cornertriangle-size-300
);
--mod-progress-circle-size: var(--spectrum-alias-workflow-icon-size-xl);
}
17 changes: 17 additions & 0 deletions packages/button/src/button.css
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,20 @@ governing permissions and limitations under the License.
border-color: highlight;
}
}

:host([pending][is-pending]) {
cursor: default;
pointer-events: none;
}

:host([pending][is-pending]) sp-progress-circle {
display: block;
position: absolute;
left: 50%;
transform: translate(-50%, 0);
Comment on lines +81 to +82
Copy link
Contributor

Choose a reason for hiding this comment

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

For reference: left here is technically correct, in that translate() doesn't really support logical properties, but CSS attempts not to use it when possible.

}

:host([pending][is-pending]) slot[name='icon'],
:host([pending][is-pending]) #label {
visibility: hidden;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't remember what visibly hiding this content does to the accessibility tree right off, so making a note to confirm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Checked in voiceover and the label seems to be hidden? Is there some other way I can test?

}
3 changes: 2 additions & 1 deletion packages/button/src/spectrum-button.css
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,8 @@ governing permissions and limitations under the License.
)
);
}
:host([disabled]) {
:host([disabled]),
:host([is-pending]) {
background-color: var(
--highcontrast-button-background-color-disabled,
var(
Expand Down
7 changes: 6 additions & 1 deletion packages/button/src/spectrum-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,14 @@ const config = {
converter.classToHost(),
converter.classToAttribute('spectrum-Button--quiet'),
converter.classToAttribute('spectrum-Button--emphasized'),
converter.classToAttribute('is-disabled', 'disabled'),
converter.classToAttribute('is-selected', 'selected'),
converter.classToAttribute('is-focused', 'focused'),
/**
* HACK!
* This relies on the fact that spectrum-css is using both `&:disabled` and `&.is-disabled` in the selectors
* for disabled states. We're using the class based selector here to also emit a `is-pending` selector.
*/
converter.classToAttribute('is-disabled', 'is-pending'),
converter.pseudoToAttribute('disabled', 'disabled'),
converter.pseudoToAttribute('active', 'active'),
converter.classToAttribute(
Expand Down
50 changes: 50 additions & 0 deletions packages/button/stories/button-accent-fill-pending.stories.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
Copyright 2020 Adobe. All rights reserved.
This file is licensed to you under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License. You may obtain a copy
of the License at http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software distributed under
the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
OF ANY KIND, either express or implied. See the License for the specific language
governing permissions and limitations under the License.
*/
import { TemplateResult } from '@spectrum-web-components/base';
import { Properties, renderButtonSet } from './index.js';
import { args, argTypes } from './index.js';

const variant = 'accent';
const treatment = 'fill';
const pending = true;

export default {
component: 'sp-button',
title: 'Button/Accent/Fill/Pending',
args: {
...args,
variant,
treatment,
pending,
},
argTypes,
};

export const s = (args: Properties): TemplateResult => renderButtonSet(args);
s.args = {
size: 's',
};

export const m = (args: Properties): TemplateResult => renderButtonSet(args);
m.args = {
size: 'm',
};

export const l = (args: Properties): TemplateResult => renderButtonSet(args);
l.args = {
size: 'l',
};

export const XL = (args: Properties): TemplateResult => renderButtonSet(args);
XL.args = {
size: 'xl',
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
Copyright 2020 Adobe. All rights reserved.
This file is licensed to you under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License. You may obtain a copy
of the License at http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software distributed under
the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
OF ANY KIND, either express or implied. See the License for the specific language
governing permissions and limitations under the License.
*/
import { TemplateResult } from '@spectrum-web-components/base';
import { Properties, renderButtonSet } from './index.js';
import { args, argTypes } from './index.js';

const variant = 'accent';
const treatment = 'outline';
const pending = true;

export default {
component: 'sp-button',
title: 'Button/Accent/Outline/Pending',
args: {
...args,
variant,
treatment,
pending,
},
argTypes,
};

export const s = (args: Properties): TemplateResult => renderButtonSet(args);
s.args = {
size: 's',
};

export const m = (args: Properties): TemplateResult => renderButtonSet(args);
m.args = {
size: 'm',
};

export const l = (args: Properties): TemplateResult => renderButtonSet(args);
l.args = {
size: 'l',
};

export const XL = (args: Properties): TemplateResult => renderButtonSet(args);
XL.args = {
size: 'xl',
};
50 changes: 50 additions & 0 deletions packages/button/stories/button-black-fill-pending.stories.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
Copyright 2020 Adobe. All rights reserved.
This file is licensed to you under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License. You may obtain a copy
of the License at http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software distributed under
the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
OF ANY KIND, either express or implied. See the License for the specific language
governing permissions and limitations under the License.
*/
import { TemplateResult } from '@spectrum-web-components/base';
import { Properties, renderButtonSet } from './index.js';
import { args, argTypes } from './index.js';

const variant = 'black';
const treatment = 'fill';
const pending = true;

export default {
component: 'sp-button',
title: 'Button/Black/Fill/Pending',
args: {
...args,
variant,
treatment,
pending,
},
argTypes,
};

export const s = (args: Properties): TemplateResult => renderButtonSet(args);
s.args = {
size: 's',
};

export const m = (args: Properties): TemplateResult => renderButtonSet(args);
m.args = {
size: 'm',
};

export const l = (args: Properties): TemplateResult => renderButtonSet(args);
l.args = {
size: 'l',
};

export const XL = (args: Properties): TemplateResult => renderButtonSet(args);
XL.args = {
size: 'xl',
};
50 changes: 50 additions & 0 deletions packages/button/stories/button-black-outline-pending.stories.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
Copyright 2020 Adobe. All rights reserved.
This file is licensed to you under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License. You may obtain a copy
of the License at http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software distributed under
the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
OF ANY KIND, either express or implied. See the License for the specific language
governing permissions and limitations under the License.
*/
import { TemplateResult } from '@spectrum-web-components/base';
import { Properties, renderButtonSet } from './index.js';
import { args, argTypes } from './index.js';

const variant = 'black';
const treatment = 'outline';
const pending = true;

export default {
component: 'sp-button',
title: 'Button/Black/Outline/Pending',
args: {
...args,
variant,
treatment,
pending,
},
argTypes,
};

export const s = (args: Properties): TemplateResult => renderButtonSet(args);
s.args = {
size: 's',
};

export const m = (args: Properties): TemplateResult => renderButtonSet(args);
m.args = {
size: 'm',
};

export const l = (args: Properties): TemplateResult => renderButtonSet(args);
l.args = {
size: 'l',
};

export const XL = (args: Properties): TemplateResult => renderButtonSet(args);
XL.args = {
size: 'xl',
};
Loading
Loading