feat(accordion): core token migration #3300

vjosyula · 2023-06-08T13:19:07Z

Description

This change covers accordion core token migration.
with this t-shirt and densities are supported on accordion.

Related issue(s)

Motivation and context

How has this been tested?

Test case 1
1. Go here
2. Do this
Test case 2
1. Go here
2. Do this

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

github-actions · 2023-06-08T13:22:30Z

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:

pfulton

Thanks for getting started with this @vjosyula.

I think the SWC team would like to see the t-shirt sizes added to this component, so if you're able to add those to the component as well as the Storybook, that would be excellent.

There are several other components in the repo that have t-shirt sizes, so you can probably take a look at those as a guide.

Westbrook · 2023-06-08T13:49:59Z

Thanks for pointing to the t-shirt size additions @pfulton that will be a very welcome addition to this PR.

Right now the VRTs are pretty unhappy, but with it still in "draft", I'm sure you're taking a look at those now that the branch is on remote.

Looking forward to seeing this come together!

vjosyula · 2023-06-08T14:26:12Z

Hi @pfulton and @Westbrook,
Working on t-shirt sizes and densities. For now, I created this PR to understand the failures.

najikahalsema

The changes look good visually, and almost everything else seems good. The only thing is the way the size attribute is handled. While you're doing that, please rebase your branch with main. That will get the github tachometer to stop failing for you on Chrome :) Keep up the great work and let me know if you have questions about anything I've written!

packages/accordion/src/Accordion.ts

packages/accordion/src/AccordionItem.ts

packages/accordion/src/spectrum-config.js

packages/accordion/stories/accordion-densities.stories.ts

github-actions · 2023-06-22T04:51:54Z

Tachometer results

Chrome

accordion permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	380 kB	186.73ms - 189.77ms	-	faster ✔ 14% - 16% 31.23ms - 36.42ms
branch	391 kB	219.97ms - 224.18ms	slower ❌ 17% - 19% 31.23ms - 36.42ms	-

action-bar permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	365 kB	35.19ms - 35.39ms	-	faster ✔ 1% - 2% 0.42ms - 0.89ms
branch	364 kB	35.74ms - 36.16ms	slower ❌ 1% - 3% 0.42ms - 0.89ms	-

action-menu permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	732 kB	174.58ms - 178.02ms	-	unsure 🔍 -2% - +1% -3.36ms - +2.56ms
branch	730 kB	174.29ms - 179.11ms	unsure 🔍 -1% - +2% -2.56ms - +3.36ms	-

card permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	485 kB	91.08ms - 92.77ms	-	unsure 🔍 -1% - +1% -1.30ms - +1.09ms
branch	484 kB	91.19ms - 92.87ms	unsure 🔍 -1% - +1% -1.09ms - +1.30ms	-

illustrated-message permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	384 kB	43.45ms - 44.14ms	-	unsure 🔍 -1% - +1% -0.33ms - +0.46ms
branch	383 kB	43.54ms - 43.92ms	unsure 🔍 -1% - +1% -0.46ms - +0.33ms	-

menu permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	416 kB	226.14ms - 231.81ms	-	unsure 🔍 -2% - +2% -4.22ms - +3.70ms
branch	415 kB	226.46ms - 232.00ms	unsure 🔍 -2% - +2% -3.70ms - +4.22ms	-

overlay permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	452 kB	71.39ms - 72.59ms	-	unsure 🔍 -1% - +2% -0.52ms - +1.13ms
branch	451 kB	71.12ms - 72.25ms	unsure 🔍 -2% - +1% -1.13ms - +0.52ms	-

picker permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	587 kB	598.21ms - 623.55ms	-	unsure 🔍 -2% - +4% -12.64ms - +22.39ms
branch	585 kB	593.91ms - 618.09ms	unsure 🔍 -4% - +2% -22.39ms - +12.64ms	-

popover permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	361 kB	33.86ms - 34.06ms	-	faster ✔ 1% - 2% 0.39ms - 0.67ms
branch	361 kB	34.39ms - 34.59ms	slower ❌ 1% - 2% 0.39ms - 0.67ms	-

slider permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	448 kB	141.83ms - 145.35ms	-	unsure 🔍 -2% - +1% -2.60ms - +2.11ms
branch	447 kB	142.27ms - 145.40ms	unsure 🔍 -1% - +2% -2.11ms - +2.60ms	-

split-button permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	665 kB	2033.08ms - 2035.61ms	-	unsure 🔍 -0% - +0% -2.26ms - +2.05ms
branch	663 kB	2032.71ms - 2036.19ms	unsure 🔍 -0% - +0% -2.05ms - +2.26ms	-

tooltip permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	356 kB	34.08ms - 34.31ms	-	unsure 🔍 -1% - +0% -0.26ms - +0.08ms
branch	355 kB	34.17ms - 34.41ms	unsure 🔍 -0% - +1% -0.08ms - +0.26ms	-

Firefox

accordion permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	380 kB	782.91ms - 821.01ms	-	slower ❌ 0% - 9% 2.48ms - 67.00ms
branch	391 kB	741.19ms - 793.25ms	faster ✔ 0% - 8% 2.48ms - 67.00ms	-

action-bar permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	365 kB	162.88ms - 191.08ms	-	faster ✔ 1% - 23% 0.90ms - 48.22ms
branch	364 kB	182.54ms - 220.54ms	unsure 🔍 -0% - +28% +0.90ms - +48.22ms	-

action-menu permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	732 kB	596.93ms - 638.43ms	-	unsure 🔍 -5% - +5% -31.75ms - +29.47ms
branch	730 kB	596.31ms - 641.33ms	unsure 🔍 -5% - +5% -29.47ms - +31.75ms	-

card permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	485 kB	304.75ms - 331.57ms	-	unsure 🔍 -8% - +7% -24.47ms - +23.43ms
branch	484 kB	298.84ms - 338.52ms	unsure 🔍 -7% - +8% -23.43ms - +24.47ms	-

illustrated-message permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	384 kB	171.74ms - 194.94ms	-	unsure 🔍 -17% - +5% -33.66ms - +10.22ms
branch	383 kB	176.44ms - 213.68ms	unsure 🔍 -6% - +19% -10.22ms - +33.66ms	-

menu permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	416 kB	770.99ms - 823.97ms	-	unsure 🔍 -3% - +5% -27.09ms - +40.53ms
branch	415 kB	769.75ms - 811.77ms	unsure 🔍 -5% - +3% -40.53ms - +27.09ms	-

overlay permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	452 kB	272.65ms - 312.39ms	-	unsure 🔍 -8% - +9% -22.88ms - +27.36ms
branch	451 kB	274.91ms - 305.65ms	unsure 🔍 -9% - +8% -27.36ms - +22.88ms	-

picker permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	587 kB	1762.53ms - 1853.11ms	-	unsure 🔍 -1% - +6% -14.43ms - +100.79ms
branch	585 kB	1729.03ms - 1800.25ms	unsure 🔍 -6% - +1% -100.79ms - +14.43ms	-

popover permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	361 kB	126.24ms - 148.60ms	-	faster ✔ 10% - 29% 15.61ms - 51.43ms
branch	361 kB	156.94ms - 184.94ms	slower ❌ 10% - 39% 15.61ms - 51.43ms	-

slider permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	448 kB	491.13ms - 524.51ms	-	unsure 🔍 -3% - +7% -13.78ms - +34.98ms
branch	447 kB	479.45ms - 514.99ms	unsure 🔍 -7% - +3% -34.98ms - +13.78ms	-

split-button permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	665 kB	2268.98ms - 2295.58ms	-	unsure 🔍 -1% - +0% -26.49ms - +9.41ms
branch	663 kB	2278.76ms - 2302.88ms	unsure 🔍 -0% - +1% -9.41ms - +26.49ms	-

tooltip permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	356 kB	132.40ms - 155.32ms	-	unsure 🔍 -8% - +15% -10.92ms - +20.60ms
branch	355 kB	128.21ms - 149.83ms	unsure 🔍 -14% - +7% -20.60ms - +10.92ms	-

pfulton · 2023-06-22T18:30:53Z

This has been merged in Spectrum CSS, and the pre-release has been graduated to the full version. The full version is: @spectrum-css/accordion@4.0.0.

najikahalsema

Just a few more things.

packages/accordion/src/Accordion.ts

packages/accordion/src/AccordionItem.ts

najikahalsema

So close. Just a few teeny changes.

packages/accordion/README.md

packages/accordion/stories/accordion.stories.ts

packages/accordion/stories/index.ts

Westbrook

Coming together quite nicely!

packages/accordion/src/Accordion.ts

packages/accordion/src/AccordionItem.ts

packages/accordion/stories/accordion-densities.stories.ts

Westbrook · 2023-06-27T13:41:10Z

This is looking really good. I'll check with @najikahalsema this afternoon that everything she was tracking has been addressed and we should be on track to merge this, soon!

Westbrook

LGTM! Thanks @venkateshjo 🙇🏼

The blocking comment has been addressed.

najikahalsema

LGTM!

vjosyula requested a review from pfulton June 8, 2023 13:19

pfulton marked this pull request as draft June 8, 2023 13:29

pfulton previously requested changes Jun 8, 2023

View reviewed changes

Westbrook requested a review from Rajdeepc June 8, 2023 13:47

vjosyula marked this pull request as ready for review June 13, 2023 14:05

pfulton requested a review from najikahalsema June 14, 2023 20:41

najikahalsema requested changes Jun 15, 2023

View reviewed changes

vjosyula force-pushed the venkateshjo/CSS-157 branch 3 times, most recently from eea00ad to 45e75c5 Compare June 22, 2023 04:22

vjosyula requested review from najikahalsema, pfulton and castastrophe June 22, 2023 06:33

najikahalsema requested changes Jun 22, 2023

View reviewed changes

vjosyula force-pushed the venkateshjo/CSS-157 branch from 9418684 to ca25b12 Compare June 23, 2023 01:43

vjosyula requested review from najikahalsema and Westbrook June 23, 2023 05:29

vjosyula force-pushed the venkateshjo/CSS-157 branch 2 times, most recently from 9679672 to e17c6b8 Compare June 26, 2023 02:44

najikahalsema requested changes Jun 26, 2023

View reviewed changes

vjosyula force-pushed the venkateshjo/CSS-157 branch from f0f3551 to ce17c1e Compare June 26, 2023 17:08

vjosyula requested a review from najikahalsema June 26, 2023 17:15

Westbrook reviewed Jun 26, 2023

View reviewed changes

venkateshjo added 2 commits June 27, 2023 05:19

feat(accordion): accordion core token migration

bcd0e81

chore(accordion): updating the storybook

03d4e2c

venkateshjo and others added 13 commits June 27, 2023 05:19

chore(accordion): minor fixes

3109bd1

chore(accordion): updating new beta package

ee80496

feat(accordion): addressing review suggestions

5c78600

chore: chevron sizing approach

ac05ef5

chore(accordion): removing ununsed code

debde21

feat(accordion): addressing review suggestions

75a4b25

feat(accordion): updating the default variables

9b21102

feat(accordion): minor fix

5f0d53a

feat(accordion): updating the documentation

7a18f54

feat(accordion): updating the storybook

ae1eb57

feat(accordion)!: updating the golden hash

8c2caaa

feat(accordion)!: removing regular references

8cfd28b

feat(accordion)!: addressing review comments

84d86fe

vjosyula force-pushed the venkateshjo/CSS-157 branch from 8eda90e to 84d86fe Compare June 26, 2023 23:50

vjosyula requested a review from Westbrook June 26, 2023 23:59

venkateshjo added 3 commits June 27, 2023 05:36

feat(accordion)!: minor lint fixes

24586ff

feat(accordion)!: minor fixes

a6c63e8

feat(accordion)!: minor fixes

80d392a

Westbrook changed the title ~~feat(accordion): accordion core token migration~~ feat(accordion): core token migration Jun 27, 2023

Westbrook approved these changes Jun 27, 2023

View reviewed changes

najikahalsema approved these changes Jun 27, 2023

View reviewed changes

najikahalsema merged commit 9650b71 into main Jun 27, 2023
47 checks passed

najikahalsema deleted the venkateshjo/CSS-157 branch June 27, 2023 17:42

Rajdeepc mentioned this pull request Jun 28, 2023

[Bug][a11y]: AccordionItem should not have role="listitem" #3212

Closed

1 task

feat(accordion): core token migration #3300

feat(accordion): core token migration #3300

Conversation

vjosyula commented Jun 8, 2023 • edited

Description

Related issue(s)

Motivation and context

How has this been tested?

Screenshots (if appropriate)

Types of changes

Checklist

github-actions bot commented Jun 8, 2023

Branch Preview

pfulton left a comment

Choose a reason for hiding this comment

Westbrook commented Jun 8, 2023

vjosyula commented Jun 8, 2023

najikahalsema left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 22, 2023 • edited

Tachometer results

accordion permalink

action-bar permalink

action-menu permalink

card permalink

illustrated-message permalink

menu permalink

overlay permalink

picker permalink

popover permalink

slider permalink

split-button permalink

tooltip permalink

accordion permalink

action-bar permalink

action-menu permalink

card permalink

illustrated-message permalink

menu permalink

overlay permalink

picker permalink

popover permalink

slider permalink

split-button permalink

tooltip permalink

pfulton commented Jun 22, 2023

najikahalsema left a comment

Choose a reason for hiding this comment

najikahalsema left a comment

Choose a reason for hiding this comment

Westbrook left a comment

Choose a reason for hiding this comment

Westbrook commented Jun 27, 2023

Westbrook left a comment

Choose a reason for hiding this comment

najikahalsema left a comment

Choose a reason for hiding this comment

vjosyula commented Jun 8, 2023 •

edited

github-actions bot commented Jun 22, 2023 •

edited