spectrum-web-components fix(picker): picker-quiet-layout

trafficstars

Description

Added a new property called sideLabel to sp-picker to introduce a new UI for sideLabel picker variant.

Related issue(s)

https://github.com/adobe/spectrum-web-components/issues/2963

Motivation and context

How has this been tested?

[ ] Test case 1
1. Go here
[ ] Test case 2
1. Go here

Screenshots (if appropriate)

Types of changes

[] Bug fix (non-breaking change which fixes an issue)
[x] 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

[x] I have signed the Adobe Open Source CLA.
[x] My code follows the code style of this project.
[x] If my change required a change to the documentation, I have updated the documentation in this pull request.
[x] I have read the CONTRIBUTING document.
[ ] I have added tests to cover my changes.
[x] All new and existing tests passed.
[x] 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.

Oct 06 '23 09:10 blunteshwar

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:

Oct 06 '23 09:10 github-actions[bot]

Tachometer results

Chrome

action-bar permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	489 kB	79.18ms - 81.31ms	-	unsure 🔍 -0% - +3% -0.30ms - +2.22ms
branch	480 kB	78.61ms - 79.97ms	unsure 🔍 -3% - +0% -2.22ms - +0.30ms	-

action-menu permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	655 kB	173.15ms - 177.49ms	-	unsure 🔍 -0% - +3% -0.32ms - +5.39ms
branch	647 kB	170.94ms - 174.64ms	unsure 🔍 -3% - +0% -5.39ms - +0.32ms	-

field-label permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	394 kB	25.20ms - 26.63ms	-	unsure 🔍 -5% - +3% -1.25ms - +0.87ms
branch	385 kB	25.32ms - 26.89ms	unsure 🔍 -3% - +5% -0.87ms - +1.25ms	-

meter permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	409 kB	57.83ms - 58.61ms	-	unsure 🔍 -0% - +2% -0.09ms - +1.06ms
branch	401 kB	57.31ms - 58.16ms	unsure 🔍 -2% - +0% -1.06ms - +0.09ms	-

picker permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	518 kB	609.84ms - 621.00ms	-	unsure 🔍 -1% - +2% -5.11ms - +10.01ms
branch	509 kB	607.87ms - 618.08ms	unsure 🔍 -2% - +1% -10.01ms - +5.11ms	-

progress-bar permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	410 kB	46.09ms - 46.70ms	-	unsure 🔍 -1% - +1% -0.67ms - +0.45ms
branch	402 kB	46.04ms - 46.97ms	unsure 🔍 -1% - +1% -0.45ms - +0.67ms	-

slider permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	481 kB	110.28ms - 112.40ms	-	unsure 🔍 -1% - +2% -0.90ms - +1.85ms
branch	472 kB	109.99ms - 111.73ms	unsure 🔍 -2% - +1% -1.85ms - +0.90ms	-

split-button permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	721 kB	1856.02ms - 1858.58ms	-	unsure 🔍 -0% - -0% -4.32ms - -0.33ms
branch	713 kB	1858.10ms - 1861.16ms	unsure 🔍 +0% - +0% +0.33ms - +4.32ms	-

Firefox

action-bar permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	489 kB	145.26ms - 151.62ms	-	unsure 🔍 -1% - +4% -2.01ms - +5.81ms
branch	480 kB	144.27ms - 148.81ms	unsure 🔍 -4% - +1% -5.81ms - +2.01ms	-

action-menu permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	655 kB	319.21ms - 331.51ms	-	unsure 🔍 -2% - +3% -6.84ms - +8.20ms
branch	647 kB	320.36ms - 329.00ms	unsure 🔍 -3% - +2% -8.20ms - +6.84ms	-

field-label permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	394 kB	57.93ms - 61.15ms	-	unsure 🔍 -7% - +3% -4.21ms - +1.61ms
branch	385 kB	58.42ms - 63.26ms	unsure 🔍 -3% - +7% -1.61ms - +4.21ms	-

meter permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	409 kB	97.94ms - 104.18ms	-	unsure 🔍 -4% - +5% -3.71ms - +4.67ms
branch	401 kB	97.78ms - 103.38ms	unsure 🔍 -5% - +4% -4.67ms - +3.71ms	-

picker permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	518 kB	1012.37ms - 1036.91ms	-	unsure 🔍 -0% - +2% -3.17ms - +23.09ms
branch	509 kB	1009.99ms - 1019.37ms	unsure 🔍 -2% - +0% -23.09ms - +3.17ms	-

progress-bar permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	410 kB	80.24ms - 84.00ms	-	unsure 🔍 -7% - +0% -6.10ms - +0.14ms
branch	402 kB	82.61ms - 87.59ms	unsure 🔍 -0% - +7% -0.14ms - +6.10ms	-

slider permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	481 kB	199.60ms - 207.88ms	-	unsure 🔍 -2% - +3% -4.42ms - +5.54ms
branch	472 kB	200.40ms - 205.96ms	unsure 🔍 -3% - +2% -5.54ms - +4.42ms	-

split-button permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	721 kB	1640.29ms - 1647.59ms	-	unsure 🔍 -0% - +1% -1.22ms - +10.34ms
branch	713 kB	1634.90ms - 1643.86ms	unsure 🔍 -1% - +0% -10.34ms - +1.22ms	-

Oct 06 '23 09:10 github-actions[bot]

Lighthouse scores

Category	Latest (report)	Main (report)	Branch (report)
Performance	0.97	0.92	0.97
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	242.858 kB	229.722 kB 🏆	230.042 kB
Scripts	59.972 kB	53.915 kB 🏆	54.102 kB
Stylesheet	50.433 kB	43.906 kB 🏆	44.099 kB
Document	5.741 kB	5.129 kB	5.129 kB
Third Party	126.712 kB	126.772 kB	126.712 kB

Request Count

Category	Latest	Main	Branch
Total	42 🏆	43	43
Scripts	34 🏆	35	35
Stylesheet	5	5	5
Document	1	1	1
Third Party	2	2	2

Feb 21 '24 09:02 github-actions[bot]

It would be great if there wasn't a requirement for both the label and the picker to notate the label location. In that way, when this is "done" then the story decorator should "just work", rather than look like:

I shared an possible path forward on this with @TarunAdobe on Tuesday, can yall sync up on that?

Feb 22 '24 14:02 Westbrook

It would be great if there wasn't a requirement for both the label and the picker to notate the label location. In that way, when this is "done" then the story decorator should "just work", rather than look like:

I shared an possible path forward on this with @TarunAdobe on Tuesday, can yall sync up on that?

I think the above css approach is a little misleading from our implementation here. The side label of picker is adjusted with display:flex on the parent which is not something we should be doing all the time. This way the picker will not know that it is rendering a side label.

Instead, we can add the sideLabel attribute to picker on demand whenever a side-aligned attribute is found on <sp-field-label>, as we are already managing its target element. A possible approach is as below

if (this.hasAttribute('side-aligned')) {
            this.resolvedElement.element?.setAttribute('sideLabel', 'true');
 }

Feb 23 '24 07:02 Rajdeepc

if (this.hasAttribute('side-aligned')) {
            this.resolvedElement.element?.setAttribute('sideLabel', 'true');
 }

Roughly, yes. This versions applied this value to everything, which may be right, but may not happen with the same attribute/property over time. What do you think about altering this type to include a second argument that is the entire Field Label class?

- applyFocusElementLabel?: (label?: string) => void;
+ applyFocusElementLabel?: (appliedLabel: string, labelElement?: FieldLabel) => void;

Then we can leverage all of the data on the FieldLabel as we see fit in the labeled element. Something like:

applyFocusElementLabel = (value: string, labelElement: FieldLabel): void => {
    this.appliedLabel = value;
    if (labelElement.sideAligned === 'start') {
       ...
    }
};

Feb 23 '24 14:02 Westbrook

FieldLabel

Yes! I would agree with this approach which would definitely detach the logic from the Field Label class. I have updated the logic above to add the attribute on the Picker level

Feb 23 '24 16:02 Rajdeepc

spectrum-web-components spectrum-web-components copied to clipboard

fix(picker): picker-quiet-layout

Description

Related issue(s)

Motivation and context

How has this been tested?

Screenshots (if appropriate)

Types of changes

Checklist

Best practices

Branch preview

Tachometer results

action-bar permalink

action-menu permalink

field-label permalink

meter permalink

picker permalink

progress-bar permalink

slider permalink

split-button permalink

action-bar permalink

action-menu permalink

field-label permalink

meter permalink

picker permalink

progress-bar permalink

slider permalink

split-button permalink

Lighthouse scores

Transfer Size

Request Count

spectrum-web-components
spectrum-web-components copied to clipboard