adobe / spectrum-web-components

Spectrum Web Components
https://opensource.adobe.com/spectrum-web-components/
Apache License 2.0
1.22k stars 191 forks source link

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

Closed Rocss closed 1 month ago

Rocss commented 1 month ago

Description

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

Spectrum CSS Screenshot 2024-05-16 at 12 10 02

Related issue(s)

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?

Screenshots (if appropriate)

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.

github-actions[bot] commented 1 month ago

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: - [High Contrast Mode | Medium | LTR](https://50dba00eebba363ad6273b954d6b55cd--spectrum-web-components.netlify.app/review/) - [Spectrum | Lightest | Medium | LTR](https://9d299ddb325865c4801f9e81048e9050--spectrum-web-components.netlify.app/review/) - [Spectrum | Lightest | Medium | RTL](https://5f230ba68756bc919b91646c2409debf--spectrum-web-components.netlify.app/review/) - [Spectrum | Lightest | Large | LTR](https://d58714ef7c99520d014efd3580c0227f--spectrum-web-components.netlify.app/review/) - [Spectrum | Lightest | Large | RTL](https://ba411250bc77dafae76ae6eccfd39302--spectrum-web-components.netlify.app/review/) - [Spectrum | Light | Medium | LTR](https://2d4ddd634c0a4b8c2a79b4f82f73c6c2--spectrum-web-components.netlify.app/review/) - [Spectrum | Light | Medium | RTL](https://43a273491c25f0f438b76b8f464320db--spectrum-web-components.netlify.app/review/) - [Spectrum | Light | Large | LTR](https://8649fa67f35010888634c814578232e9--spectrum-web-components.netlify.app/review/) - [Spectrum | Light | Large | RTL](https://995349eb1f6d76208cacad1350b10809--spectrum-web-components.netlify.app/review/) - [Spectrum | Dark | Medium | LTR](https://0c94b268ed77ac8c4b40c11f6c6fe7e7--spectrum-web-components.netlify.app/review/) - [Spectrum | Dark | Medium | RTL](https://1c23d3f2b570e1a6b3c62b5002ab8984--spectrum-web-components.netlify.app/review/) - [Spectrum | Dark | Large | LTR](https://bdd23a940df2a45905de7d63239091e9--spectrum-web-components.netlify.app/review/) - [Spectrum | Dark | Large | RTL](https://f8d6f869621f89a4ac48fce67f8d417b--spectrum-web-components.netlify.app/review/) - [Spectrum | Darkest | Medium | LTR](https://1eeedb1ddb9fe846e2d2c95f487381ef--spectrum-web-components.netlify.app/review/) - [Spectrum | Darkest | Medium | RTL](https://cd44c7ee12cf281a0a11c81d84efb2d2--spectrum-web-components.netlify.app/review/) - [Spectrum | Darkest | Large | LTR](https://43f780816cfadcc1d02ca13fb3eb3340--spectrum-web-components.netlify.app/review/) - [Spectrum | Darkest | Large | RTL](https://584938e7cb980cde927dbb49b86e6049--spectrum-web-components.netlify.app/review/) - [Express | Lightest | Medium | LTR](https://fabdd4d1f0f77a68331d7bdcb4cb83c6--spectrum-web-components.netlify.app/review/) - [Express | Lightest | Medium | RTL](https://e798654fb054a4e25b35ccc980d863ce--spectrum-web-components.netlify.app/review/) - [Express | Lightest | Large | LTR](https://40a84ccebb47d97d4f859f035a55d713--spectrum-web-components.netlify.app/review/) - [Express | Lightest | Large | RTL](https://f2b7ab3875e87ceb90dac2f05f53d0dc--spectrum-web-components.netlify.app/review/) - [Express | Light | Medium | LTR](https://842efb8c6fae9842798dfc676c790c8c--spectrum-web-components.netlify.app/review/) - [Express | Light | Medium | RTL](https://322c11352a234d0adcac378e69e10311--spectrum-web-components.netlify.app/review/) - [Express | Light | Large | LTR](https://9ec49fff3230a5e27753f25d645ee6c0--spectrum-web-components.netlify.app/review/) - [Express | Light | Large | RTL](https://aab9d738f1eb0070a0a1e6d750987e90--spectrum-web-components.netlify.app/review/) - [Express | Dark | Medium | LTR](https://487a7a1c5bc067692ff2b6b5f1508b9c--spectrum-web-components.netlify.app/review/) - [Express | Dark | Medium | RTL](https://430bd791a646d3005665767c63857d76--spectrum-web-components.netlify.app/review/) - [Express | Dark | Large | LTR](https://d7177f2225442e13282c8e40061a7acc--spectrum-web-components.netlify.app/review/) - [Express | Dark | Large | RTL](https://6a3dad03757fcd6e043d4876c8b94c61--spectrum-web-components.netlify.app/review/) - [Express | Darkest | Medium | LTR](https://1246a9f37fd04159157df2ca273149a4--spectrum-web-components.netlify.app/review/) - [Express | Darkest | Medium | RTL](https://942c21d1869a95e874c90100c1ad2cec--spectrum-web-components.netlify.app/review/) - [Express | Darkest | Large | LTR](https://1cc3cfc06ea851cd2e0dd19ff8ea56b0--spectrum-web-components.netlify.app/review/) - [Express | Darkest | Large | RTL](https://7ab682bf7ec0593d865a5a72d136d007--spectrum-web-components.netlify.app/review/) - [Spectrum-two | Light | Medium | LTR](https://9f21f1d997c8ad4ef24f6b2cdd52595e--spectrum-web-components.netlify.app/review/) - [Spectrum-two | Light | Medium | RTL](https://2a3de8175a74905682cb8ba483c3124a--spectrum-web-components.netlify.app/review/) - [Spectrum-two | Light | Large | LTR](https://91f1d6032b5bcfbab1399397eff74acd--spectrum-web-components.netlify.app/review/) - [Spectrum-two | Light | Large | RTL](https://d309169bf96711422c6043f5e51a30da--spectrum-web-components.netlify.app/review/) - [Spectrum-two | Dark | Medium | LTR](https://3f0edd998742565c462cad5f275bf9b1--spectrum-web-components.netlify.app/review/) - [Spectrum-two | Dark | Medium | RTL](https://9f36bdcb57dbc895c8b267a4bf429239--spectrum-web-components.netlify.app/review/) - [Spectrum-two | Dark | Large | LTR](https://df3b3441a00760dfbe3c3e60a98dd404--spectrum-web-components.netlify.app/review/) - [Spectrum-two | Dark | Large | RTL](https://7d60e309f41dbdc0938e3a95a3fdfb30--spectrum-web-components.netlify.app/review/)
github-actions[bot] commented 1 month ago

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](https://github.com/GoogleChrome/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.482 kB 210.775 kB πŸ† 210.778 kB
Scripts 53.548 kB 48.466 kB 48.372 kB πŸ†
Stylesheet 35.004 kB 30.368 kB πŸ† 30.504 kB
Document 5.981 kB 5.335 kB 5.269 kB πŸ†
Font 126.949 kB 126.606 kB πŸ† 126.633 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
github-actions[bot] commented 1 month ago

Tachometer results

Chrome ## combobox [_permalink_](#user-content-combobox) ### basic-test | Version | Bytes | Avg Time | vs remote | vs branch | |---|---|---|---|---| | npm latest | 709 kB | 35.63ms - 36.22ms | - | faster βœ”
5% - 7%
1.91ms - 2.72ms | | branch | 697 kB | 37.97ms - 38.52ms | slower ❌
5% - 8%
1.91ms - 2.72ms | - | ### light-dom-test [_permalink_](#user-content-combobox-light-dom-test) | Version | Bytes | Avg Time | vs remote | vs branch | |---|---|---|---|---| | npm latest | 710 kB | 394.66ms - 404.23ms | - | faster βœ”
1% - 4%
3.75ms - 15.36ms | | branch | 698 kB | 405.71ms - 412.29ms | slower ❌
1% - 4%
3.75ms - 15.36ms | - |
Firefox ## combobox [_permalink_](#user-content-combobox) ### basic-test | Version | Bytes | Avg Time | vs remote | vs branch | |---|---|---|---|---| | npm latest | 709 kB | 60.15ms - 63.57ms | - | unsure πŸ”
-3% - +3%
-1.92ms - +1.96ms | | branch | 697 kB | 60.93ms - 62.75ms | unsure πŸ”
-3% - +3%
-1.96ms - +1.92ms | - | ### light-dom-test [_permalink_](#user-content-combobox-light-dom-test) | Version | Bytes | Avg Time | vs remote | vs branch | |---|---|---|---|---| | npm latest | 710 kB | 725.04ms - 735.00ms | - | slower ❌
1% - 4%
5.79ms - 27.01ms | | branch | 698 kB | 704.25ms - 722.99ms | faster βœ”
1% - 4%
5.79ms - 27.01ms | - |
Rajdeepc commented 1 month ago

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

Screenshot 2024-05-20 at 11 41 42β€―AM
TarunAdobe commented 1 month ago

Should a readonly combobox accept a pending state?

image
Rajdeepc commented 1 month ago

Should a readonly combobox accept a pending state? image

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

Rocss commented 1 month ago

Should a readonly combobox accept a pending state? image

@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 commented 1 month ago

Should a readonly combobox accept a pending state? image

@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!