Skip to content

Commit

Permalink
fix(picker): add loading state to the picker (#3110)
Browse files Browse the repository at this point in the history
* fix(picker): add loading state to the picker

* chore(picker): implement the code review

* chore(picker): update circleci golden image hash

* chore(picker): tests + a11y

* chore(picker): create pendingLabel property and update a bunch of small stuff

* Update config.yml

* chore(picker): implement code review

* chore(picker): remove unnecessary size on sp-progress-circle

* chore(picker): remove custom sizes for spinner

* chore(picker): use aria-valuetext to bypass firefox issue

* chore(picker): remove unnecessary old forgotten code

* chore(picker): update golden image cache

* Merge branch 'main' into razvancir96/add-picker-loading-state

* chore: implement feedback on PR

* chore: add spinner to package.json

* chore: golden image cache

---------

Co-authored-by: rcirlugea <rcirlugea@adobe.com>
Co-authored-by: rmanole <rmanole@adobe.com>
Co-authored-by: Roxana Burduja <13311865+Rocss@users.noreply.github.com>
  • Loading branch information
4 people authored Mar 12, 2024
1 parent 287b979 commit d91e2c9
Show file tree
Hide file tree
Showing 15 changed files with 204 additions and 22 deletions.
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: d95e1f5e29f6a847ce8190ad6ad6028456707b71
default: 3ac2a24425678f3fbe475bee63696d063a4c3331
wireit_cache_name:
type: string
default: wireit
Expand Down
2 changes: 1 addition & 1 deletion RELEASE_PROCESS.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Users with permissions in the `@spectrum-web-components` organization on NPM can
1. Merge all pull requests to be included in the release and wait for the `main` branch to show that it has completed the required CI jobs.
2. `git checkout main && git fetch && git pull && git clean -dfX`
3. Run `nvm use` assumes a Node Version Manager install, and confirm your on an operable version of Node.
4. `yarn install`
4. `yarn install`
5. `npm whoami` ensure that you are logged in with the user account for the public NPM registry
6. `yarn lerna-publish`
7. Scan the version summary for any unexpected changes.
Expand Down
12 changes: 2 additions & 10 deletions packages/overlay/stories/overlay.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,7 @@ const template = ({
type=${ifDefined(type)}
>
<sp-button variant="primary" slot="trigger">Show Popover</sp-button>
<sp-popover
slot="click-content"
placement="${placement}"
tip
>
<sp-popover slot="click-content" placement="${placement}" tip>
<sp-dialog no-divider>
<sp-slider
value="5"
Expand All @@ -181,11 +177,7 @@ const template = ({
</div>
<overlay-trigger id="inner-trigger" placement="bottom">
<sp-button slot="trigger">Press Me</sp-button>
<sp-popover
slot="click-content"
placement="bottom"
tip
>
<sp-popover slot="click-content" placement="bottom" tip>
<sp-dialog size="s" no-divider>
Another Popover
</sp-dialog>
Expand Down
30 changes: 29 additions & 1 deletion packages/picker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ When the `value` of an `<sp-picker>` matches the `value` attribute or the trimme
</sp-picker>
<br />
<br />
<sp-field-label for="picker-disabled">Quiet:</sp-field-label>
<sp-field-label for="picker-disabled-quiet">Quiet:</sp-field-label>
<sp-picker
label="Select a Country with a very long label, too long in fact"
disabled
Expand All @@ -431,6 +431,34 @@ When the `value` of an `<sp-picker>` matches the `value` attribute or the trimme
</sp-picker>
```

### Pending

When in pending state, `<sp-picker>` elements will not respond to click events and will be delivered with a `<sp-progress-circle>` to visually outline that it is pending. It will not toggle open or display its `<sp-menu-item>` descendants until the attribute is removed.

```html
<sp-field-label for="picker-loading">Standard:</sp-field-label>
<sp-picker id="picker-loading" label="Loading blending modes..." pending>
<sp-menu-item>Pass through</sp-menu-item>
<sp-menu-item>Normal</sp-menu-item>
<sp-menu-item>Multiply</sp-menu-item>
<sp-menu-item>Screen</sp-menu-item>
</sp-picker>
<br />
<br />
<sp-field-label for="picker-loading-quiet">Quiet:</sp-field-label>
<sp-picker
id="picker-loading-quiet"
label="Loading blending modes..."
pending
quiet
>
<sp-menu-item>Pass through</sp-menu-item>
<sp-menu-item>Normal</sp-menu-item>
<sp-menu-item>Multiply</sp-menu-item>
<sp-menu-item>Screen</sp-menu-item>
</sp-picker>
```

## Accessibility

To render accessibly, an `<sp-picker>` element should be paired with an `<sp-field-label>` element that has a `for` attribute referencing the `id` of the `<sp-picker>` element. For an accessible label that renders within the bounds of the picker itself, but still fulfills the accessibility contract, use the `label` attribute or a `<span slot="label">` as a child element of `<sp-picker>`.
1 change: 1 addition & 0 deletions packages/picker/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
"@spectrum-web-components/menu": "^0.41.2",
"@spectrum-web-components/overlay": "^0.41.2",
"@spectrum-web-components/popover": "^0.41.2",
"@spectrum-web-components/progress-circle": "^0.41.2",
"@spectrum-web-components/reactive-controllers": "^0.41.2",
"@spectrum-web-components/shared": "^0.41.2",
"@spectrum-web-components/tooltip": "^0.41.2",
Expand Down
35 changes: 31 additions & 4 deletions packages/picker/src/Picker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
ifDefined,
StyleInfo,
styleMap,
when,
} from '@spectrum-web-components/base/src/directives.js';
import {
property,
Expand Down Expand Up @@ -83,6 +84,14 @@ export class PickerBase extends SizedMixin(Focusable, { noDefaultSize: true }) {
@property({ type: Boolean, reflect: true })
public invalid = false;

/** Whether the items are currently loading. */
@property({ type: Boolean, reflect: true })
public pending = false;

/** Defines a string value that labels the Picker while it is in pending state. */
@property({ type: String, attribute: 'pending-label' })
public pendingLabel = 'Pending';

@property()
public label?: string;

Expand Down Expand Up @@ -316,7 +325,7 @@ export class PickerBase extends SizedMixin(Focusable, { noDefaultSize: true }) {
}

public toggle(target?: boolean): void {
if (this.readonly) {
if (this.readonly || this.pending) {
return;
}
this.open = typeof target !== 'undefined' ? target : !this.open;
Expand Down Expand Up @@ -440,13 +449,28 @@ export class PickerBase extends SizedMixin(Focusable, { noDefaultSize: true }) {
: html`
<span hidden id="applied-label">${appliedLabel}</span>
`}
${this.invalid
${this.invalid && !this.pending
? html`
<sp-icon-alert
class="validation-icon"
></sp-icon-alert>
`
: nothing}
${when(this.pending, () => {
import(
'@spectrum-web-components/progress-circle/sp-progress-circle.js'
);
// aria-valuetext is a workaround for aria-valuenow being applied in Firefox even in indeterminate mode.
return html`
<sp-progress-circle
id="loader"
size="s"
indeterminate
aria-valuetext=${this.pendingLabel}
class="progress-circle"
></sp-progress-circle>
`;
})}
<sp-icon-chevron100
class="picker ${chevronClass[
this.size as DefaultElementSize
Expand Down Expand Up @@ -517,7 +541,7 @@ export class PickerBase extends SizedMixin(Focusable, { noDefaultSize: true }) {
aria-describedby="tooltip"
aria-expanded=${this.open ? 'true' : 'false'}
aria-haspopup="true"
aria-labelledby="icon label applied-label"
aria-labelledby="loader icon label applied-label"
id="button"
class=${ifDefined(
this.labelAlignment
Expand Down Expand Up @@ -550,6 +574,9 @@ export class PickerBase extends SizedMixin(Focusable, { noDefaultSize: true }) {
if (changes.has('disabled') && this.disabled) {
this.open = false;
}
if (changes.has('pending') && this.pending) {
this.open = false;
}
if (changes.has('value')) {
// MenuItems update a frame late for <slot> management,
// await the same here.
Expand Down Expand Up @@ -843,7 +870,7 @@ export class Picker extends PickerBase {
protected override handleKeydown = (event: KeyboardEvent): void => {
const { code } = event;
this.focused = true;
if (!code.startsWith('Arrow') || this.readonly) {
if (!code.startsWith('Arrow') || this.readonly || this.pending) {
return;
}
if (code === 'ArrowUp' || code === 'ArrowDown') {
Expand Down
2 changes: 1 addition & 1 deletion packages/picker/src/picker.css
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ sp-menu {
margin-inline-start: auto;
}

:host([focused]:not([quiet])) #button .picker {
:host([focused]:not([quiet], [pending])) #button .picker {
/* .spectrum-Picker-trigger.focus-ring .spectrum-Picker-icon */
color: var(
--spectrum-picker-icon-color-key-focus,
Expand Down
5 changes: 5 additions & 0 deletions packages/picker/src/spectrum-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const config = {
converter.classToId('spectrum-Picker', 'button'),
converter.classToAttribute('spectrum-Picker--quiet'),
converter.classToAttribute('is-disabled', 'disabled'),
converter.classToAttribute('is-loading', 'pending'),
converter.classToAttribute('is-invalid', 'invalid'),
converter.classToAttribute('is-open', 'open'),
converter.classToAttribute('is-focused', 'focused'),
Expand All @@ -51,6 +52,10 @@ const config = {
'label-inline'
),
converter.classToClass('spectrum-Menu-checkmark', 'checkmark'),
converter.classToClass(
'spectrum-ProgressCircle',
'progress-circle'
),
converter.classToClass('is-placeholder', 'placeholder'),
converter.classToClass(
'spectrum-Picker-validationIcon',
Expand Down
6 changes: 3 additions & 3 deletions packages/picker/src/spectrum-picker.css
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ governing permissions and limitations under the License.
);
}

#button.is-loading .picker {
:host([pending]) #button .picker {
color: var(
--highcontrast-picker-content-color-disabled,
var(
Expand Down Expand Up @@ -848,7 +848,7 @@ governing permissions and limitations under the License.
}

.validation-icon,
#button .spectrum-ProgressCircle {
#button .progress-circle {
margin-inline-start: var(
--mod-picker-spacing-text-to-alert-icon-inline-start,
var(--spectrum-picker-spacing-text-to-alert-icon-inline-start)
Expand All @@ -872,7 +872,7 @@ governing permissions and limitations under the License.
);
}

#button .spectrum-ProgressCircle {
#button .progress-circle {
margin-block-start: calc(
var(
--mod-picker-spacing-top-to-progress-circle,
Expand Down
1 change: 1 addition & 0 deletions packages/picker/stories/args.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export const argTypes = {
},
type: 'select',
},
options: ['s', 'm', 'l', 'xl'],
},
quiet: {
name: 'quiet',
Expand Down
36 changes: 36 additions & 0 deletions packages/picker/stories/picker-pending.stories.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
Copyright 2023 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 { argTypes } from './args';
import { StoryArgs, Template } from './template';

export default {
title: 'Picker/Pending',
component: 'sp-picker',
argTypes,
args: {
pending: true,
},
};

export const S = (args: StoryArgs): TemplateResult =>
Template({ ...args, size: 's' });

export const M = (args: StoryArgs): TemplateResult =>
Template({ ...args, size: 'm' });

export const L = (args: StoryArgs): TemplateResult =>
Template({ ...args, size: 'l' });

export const XL = (args: StoryArgs): TemplateResult =>
Template({ ...args, size: 'xl' });
30 changes: 30 additions & 0 deletions packages/picker/stories/picker-sizes.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,49 @@ export default {
component: 'sp-picker',
argTypes: {
onChange: { action: 'change' },
invalid: {
name: 'invalid',
type: { name: 'boolean', required: false },
table: {
type: { summary: 'boolean' },
defaultValue: { summary: false },
},
control: {
type: 'boolean',
},
},
pending: {
name: 'pending',
type: { name: 'boolean', required: false },
table: {
type: { summary: 'boolean' },
defaultValue: { summary: false },
},
control: {
type: 'boolean',
},
},
},
};

type StoryArgs = {
onChange: (val: string) => void;
invalid: boolean;
pending: boolean;
open: false;
};

const picker = ({
onChange,
open,
size,
pending,
invalid,
}: {
onChange: (val: string) => void;
size: 's' | 'm' | 'l' | 'xl';
pending: boolean;
invalid: boolean;
open: boolean;
}): TemplateResult => {
return html`
Expand All @@ -51,6 +79,8 @@ const picker = ({
onChange(picker.value);
}}"
label="Select a Country with a very long label, too long, in fact"
?pending="${pending}"
?invalid="${invalid}"
?open=${open}
>
<sp-menu-item>Deselect</sp-menu-item>
Expand Down
19 changes: 18 additions & 1 deletion packages/picker/stories/picker.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@ export default {
invalid: false,
open: false,
quiet: false,
pending: false,
},
argTypes: {
...argTypes,
onChange: { action: 'change' },
open: {
name: 'open',
Expand All @@ -48,7 +50,17 @@ export default {
},
control: 'boolean',
},
...argTypes,
pending: {
name: 'pending',
type: { name: 'boolean', required: false },
table: {
type: { summary: 'boolean' },
defaultValue: { summary: false },
},
control: {
type: 'boolean',
},
},
},
};

Expand Down Expand Up @@ -85,6 +97,11 @@ disabled.args = {
disabled: true,
};

export const invalid = (args: StoryArgs): TemplateResult => Template(args);
invalid.args = {
invalid: true,
};

export const tooltip = (args: StoryArgs): TemplateResult => {
const { open, ...rest } = args;
return html`
Expand Down
1 change: 1 addition & 0 deletions packages/picker/stories/template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export interface StoryArgs {
invalid?: boolean;
open?: boolean;
quiet?: boolean;
pending?: boolean;
showText?: boolean;
onChange?: (val: string) => void;
[prop: string]: unknown;
Expand Down
Loading

0 comments on commit d91e2c9

Please sign in to comment.