feat(combobox): add `pending` state #4462

Rocss · 2024-05-16T09:27:57Z

Description

Adds pending property for sp-combobox, enabling the pending state.

Spectrum CSS

Related issue(s)

[Feat]: Support pending state for Combobox #4382

Motivation and context

A combo box can indicate that content is loading if system processes delay the display of the combo box content.

How has this been tested?

Test case 1
1. Go here
2. Observe documentation entries for quiet and pending states are now available
Test case 2
1. Go here
2. Combobox should receive focus when in pending state
3. Your are able to type inside it, however, it does not open the suggestion list while it is pending
4. Toggling the open control does not open the suggestion list
Test case 3
1. Go here
2. Inspect the combobox using Screen Reader (I used VoiceOver on Mac)
3. It reads the "Pending" label together with the Combobox's visible label.
Test case 4
1. Go here
2. Toggle between LTR / RTL
3. The progress circle is correctly positioned near the picker-button
Test case 5
1. Go here
2. Enable disabled control -> all looks good
Test case 6
1. Go here
2. Enable invalid control -> progress circle takes precedence over the alert icon
Test case 7
1. Go here
2. Enable quiet control -> all looks good

Screenshots (if appropriate)

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Checklist

I have signed the Adobe Open Source CLA.
My code follows the code style of this project.
If my change required a change to the documentation, I have updated the documentation in this pull request.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.
I have reviewed at the Accessibility Practices for this feature, see: Aria Practices

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.

github-actions · 2024-05-16T09:30:05Z

Branch preview

Visual regression test results

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

github-actions · 2024-05-16T09:32:17Z

Lighthouse scores

Category	Latest (report)	Main (report)	Branch (report)
Performance	0.99	0.99	0.99
Accessibility	1	1	1
Best Practices	1	1	1
SEO	1	0.92	0.92
PWA	1	1	1

What is this?

Lighthouse scores comparing the documentation site built from the PR ("Branch") to that of the production documentation site ("Latest") and the build currently on main ("Main"). Higher scores are better, but note that the SEO scores on Netlify URLs are artifically constrained to 0.92.

Transfer Size

Category	Latest	Main	Branch
Total	221.663 kB	210.513 kB 🏆	210.594 kB
Scripts	53.795 kB	48.286 kB	48.262 kB 🏆
Stylesheet	35.007 kB	30.365 kB 🏆	30.514 kB
Document	5.982 kB	5.266 kB	5.195 kB 🏆
Font	126.879 kB	126.596 kB 🏆	126.623 kB

Request Count

Category	Latest	Main	Branch
Total	45	45	45
Scripts	37	37	37
Stylesheet	5	5	5
Document	1	1	1
Font	2	2	2

Rocss · 2024-05-16T09:47:52Z

packages/combobox/src/Combobox.ts

@@ -402,6 +440,7 @@ export class Combobox extends Textfield {
                    : ''}"
                ?disabled=${this.disabled}
                ?focused=${this.focused}
+                ?quiet=${this.quiet}


not really related to this PR but a very straight-forward fix for the quiet state

github-actions · 2024-05-16T10:08:15Z

Tachometer results

Chrome

color-field permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	487 kB	43.52ms - 45.02ms	-	faster ✔ 2% - 7% 0.69ms - 3.17ms
branch	475 kB	45.21ms - 47.18ms	slower ❌ 2% - 7% 0.69ms - 3.17ms	-

combobox permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	709 kB	38.94ms - 39.53ms	-	faster ✔ 6% - 9% 2.38ms - 3.94ms
branch	698 kB	41.67ms - 43.12ms	slower ❌ 6% - 10% 2.38ms - 3.94ms	-

light-dom-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	710 kB	436.21ms - 442.98ms	-	faster ✔ 2% - 4% 7.32ms - 16.79ms
branch	698 kB	448.34ms - 454.97ms	slower ❌ 2% - 4% 7.32ms - 16.79ms	-

number-field permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	523 kB	71.12ms - 72.58ms	-	faster ✔ 9% - 12% 7.22ms - 10.13ms
branch	511 kB	79.27ms - 81.79ms	slower ❌ 10% - 14% 7.22ms - 10.13ms	-

search permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	472 kB	40.66ms - 41.39ms	-	faster ✔ 4% - 6% 1.52ms - 2.70ms
branch	459 kB	42.67ms - 43.60ms	slower ❌ 4% - 7% 1.52ms - 2.70ms	-

slider permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	480 kB	79.48ms - 80.92ms	-	faster ✔ 3% - 6% 2.66ms - 5.18ms
branch	467 kB	83.09ms - 85.15ms	slower ❌ 3% - 6% 2.66ms - 5.18ms	-

textfield permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	432 kB	24.92ms - 25.46ms	-	faster ✔ 10% - 13% 2.93ms - 3.85ms
branch	420 kB	28.20ms - 28.95ms	slower ❌ 12% - 15% 2.93ms - 3.85ms	-

Firefox

color-field permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	495 kB	77.32ms - 80.40ms	-	unsure 🔍 -4% - +1% -3.13ms - +0.57ms
branch	483 kB	79.13ms - 81.15ms	unsure 🔍 -1% - +4% -0.57ms - +3.13ms	-

combobox permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	709 kB	62.46ms - 68.82ms	-	slower ❌ 2% - 13% 1.13ms - 7.67ms
branch	698 kB	60.50ms - 61.98ms	faster ✔ 2% - 11% 1.13ms - 7.67ms	-

light-dom-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	710 kB	720.88ms - 730.40ms	-	slower ❌ 0% - 4% 0.74ms - 26.58ms
branch	698 kB	699.97ms - 723.99ms	faster ✔ 0% - 4% 0.74ms - 26.58ms	-

number-field permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	523 kB	146.64ms - 153.80ms	-	faster ✔ 5% - 10% 7.42ms - 16.54ms
branch	511 kB	159.37ms - 165.03ms	slower ❌ 5% - 11% 7.42ms - 16.54ms	-

search permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	472 kB	69.85ms - 75.27ms	-	unsure 🔍 -9% - +1% -7.05ms - +0.49ms
branch	459 kB	73.22ms - 78.46ms	unsure 🔍 -1% - +10% -0.49ms - +7.05ms	-

slider permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	480 kB	163.99ms - 171.17ms	-	faster ✔ 1% - 7% 1.26ms - 12.94ms
branch	467 kB	170.07ms - 179.29ms	slower ❌ 1% - 8% 1.26ms - 12.94ms	-

textfield permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	432 kB	47.91ms - 52.01ms	-	unsure 🔍 -8% - +3% -4.11ms - +1.39ms
branch	420 kB	49.48ms - 53.16ms	unsure 🔍 -3% - +8% -1.39ms - +4.11ms	-

packages/combobox/src/Combobox.ts

Rajdeepc · 2024-05-20T06:13:07Z

This case shouldn't surface up!
In disabled state pending state should be undefined. Please confirm with design. Thanks

TarunAdobe · 2024-05-20T06:22:39Z

Should a readonly combobox accept a pending state?

Rajdeepc · 2024-05-20T11:43:28Z

Should a readonly combobox accept a pending state?

https://spectrum.adobe.com/page/combo-box/ needs to be updated with pending state. I will convey it to the design team.

packages/combobox/test/combobox.test.ts

Rocss · 2024-05-21T11:12:22Z

Should a readonly combobox accept a pending state?

@TarunAdobe mmmm in my opinion no, but I can not find any design docs on this :( Let's not allow pending if readonly, so that this is consistent with the sp-picker component for now. What do you think?

@Rajdeepc Agree with the disabled and pending not really working at the same time. Seems like in the sp-picker I allowed disabled and pending at the same time. Given that there are no design docs for now on this, should be enforce disabled to have a higher priority here?

Rajdeepc · 2024-05-24T11:52:55Z

Should a readonly combobox accept a pending state?

@TarunAdobe mmmm in my opinion no, but I can not find any design docs on this :( Let's not allow pending if readonly, so that this is consistent with the sp-picker component for now. What do you think?

@Rajdeepc Agree with the disabled and pending not really working at the same time. Seems like in the sp-picker I allowed disabled and pending at the same time. Given that there are no design docs for now on this, should be enforce disabled to have a higher priority here?

I have already send out a note to design to confirm this but yes if it is disabled, pending state should not be visible. It is also not accessible complaint. Let me know if you have any other questions!

rmanole added 4 commits May 16, 2024 11:43

chore: add pending state to combobox

27bb768

chore: adds tests, a11y, docs, refactor stories

c2f6bb7

chore: fix typo

4beb83c

chore: fix a11y test

3630c63

chore: golden image cache

ffbb248

Rocss commented May 16, 2024

View reviewed changes

Rocss linked an issue May 16, 2024 that may be closed by this pull request

[Feat]: Support pending state for Combobox #4382

Open

1 task

Rocss marked this pull request as ready for review May 16, 2024 10:33

Rocss requested a review from a team May 16, 2024 10:33

Rajdeepc requested changes May 17, 2024

View reviewed changes

packages/combobox/src/Combobox.ts Outdated Show resolved Hide resolved

Rajdeepc requested a review from a team May 20, 2024 05:25

Westbrook reviewed May 20, 2024

View reviewed changes

packages/combobox/test/combobox.test.ts Outdated Show resolved Hide resolved

rmanole added 2 commits May 27, 2024 11:24

Merge branch 'main' into rocss/pending-combobox

d1669ac

chore: do not allow pending state for disabled and readonly

a206358

Rocss requested a review from Rajdeepc May 27, 2024 08:54

Rajdeepc added the Component: Combobox label May 30, 2024

feat(combobox): add pending state #4462

Are you sure you want to change the base?

feat(combobox): add pending state #4462

Conversation

Rocss commented May 16, 2024 • edited

Description

Related issue(s)

Motivation and context

How has this been tested?

Screenshots (if appropriate)

Types of changes

Checklist

Best practices

github-actions bot commented May 16, 2024

Branch preview

github-actions bot commented May 16, 2024 • edited

Lighthouse scores

Transfer Size

Request Count

Rocss May 16, 2024

Choose a reason for hiding this comment

github-actions bot commented May 16, 2024 • edited

Tachometer results

color-field permalink

basic-test

combobox permalink

basic-test

light-dom-test permalink

number-field permalink

basic-test

search permalink

test-basic

slider permalink

test-basic

textfield permalink

test-basic

color-field permalink

basic-test

combobox permalink

basic-test

light-dom-test permalink

number-field permalink

basic-test

search permalink

test-basic

slider permalink

test-basic

textfield permalink

test-basic

Rajdeepc commented May 20, 2024 • edited

TarunAdobe commented May 20, 2024

Rajdeepc commented May 20, 2024

Rocss commented May 21, 2024 • edited

Rajdeepc commented May 24, 2024

feat(combobox): add `pending` state #4462

feat(combobox): add `pending` state #4462

Rocss commented May 16, 2024 •

edited

github-actions bot commented May 16, 2024 •

edited

github-actions bot commented May 16, 2024 •

edited

Rajdeepc commented May 20, 2024 •

edited

Rocss commented May 21, 2024 •

edited