feat(picker): add loading state to the picker #3093
Conversation
| --spectrum-progress-circle-thickness: var( | ||
| --spectrum-progress-circle-thickness-small | ||
| ); | ||
| } |
There was a problem hiding this comment.
Should I pick some other variables? Or make my own ones? More context: the picker has 4 dimensions: s/m/l/xl and the loading spinner has 3 dimensions: s/m/l, and do not match together. So I had to adapt its dimensions. Also, should we validate with design those dimensions?
There was a problem hiding this comment.
@pfulton is there any token support for these variation?
There was a problem hiding this comment.
From a selector standpoint, we can use :host([size="s"]) sp-progress-circle instead of needing to add a dynamic class to this element.
|
|
||
| expect(el.loading).to.be.true; | ||
| await expect(el).to.be.accessible(); | ||
| }); |
There was a problem hiding this comment.
I copied the pattern from the invalid property, but I feel like some more tests should be added. Are there any guidelines about writing tests?
There was a problem hiding this comment.
Two things we'll want to test for:
- is the listbox disabled when
loading: load, apply loading, try to click, see thatopen === falseand the Menu Items cannot be clicked withsendMouse - is the final "loading" label applied to the accessibility tree: load, apply loading, get an accessibility snapshot, ensure there is a node with the expected label
There was a problem hiding this comment.
Thanks! One additional question about testing: is there any command to run only the tests from one file? I tried some combinations, but failed 😅
There was a problem hiding this comment.
yarn test:watch:focus picker
| --spectrum-progress-circle-thickness: var( | ||
| --spectrum-progress-circle-thickness-small | ||
| ); | ||
| } |
There was a problem hiding this comment.
@pfulton is there any token support for these variation?
| --spectrum-progress-circle-thickness: var( | ||
| --spectrum-progress-circle-thickness-small | ||
| ); | ||
| } |
There was a problem hiding this comment.
From a selector standpoint, we can use :host([size="s"]) sp-progress-circle instead of needing to add a dynamic class to this element.
|
|
||
| expect(el.loading).to.be.true; | ||
| await expect(el).to.be.accessible(); | ||
| }); |
There was a problem hiding this comment.
Two things we'll want to test for:
- is the listbox disabled when
loading: load, apply loading, try to click, see thatopen === falseand the Menu Items cannot be clicked withsendMouse - is the final "loading" label applied to the accessibility tree: load, apply loading, get an accessibility snapshot, ensure there is a node with the expected label
2 participants
Description
We need to add a loading spinner inside the picker's button.
Related issue(s)
Motivation and context
In one of Adobe's projects, we need to support the loading spinner inside the picker's button.
How has this been tested?
yarn storybookloadingproperty on true. Also, change the scale.Screenshots (if appropriate)
Size S:
Size M:
Size L:
Size XL:
Types of changes
Checklist
Best practices
This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against
main.