feat: migrate asset to s2 styles and second-gen architecture #5858

TarunAdobe · 2025-11-06T09:01:58Z

Description

This PR migrates the next-gen Asset component to Spectrum 2 styles and updates Storybook accordingly.

Motivation and context

This PR migrates the Asset component from first-gen to the second-gen architecture, following the established pattern of separating core component logic from Spectrum-specific implementation.

Changes:

Migrated CSS to align with Spectrum CSS structure

What changed

First-gen:

first-gen/packages/asset/src/Asset.ts - Remains unchanged for backward compatibility

Core (Base):

second-gen/packages/core/components/asset/Asset.base.ts - Updates shared functionalities
second-gen/packages/core/components/asset/Asset.types.ts - Creates types file for size and static color variants
second-gen/packages/core/components/asset/index.ts - Imports and exports types

Second-gen:

second-gen/packages/swc/components/asset/Asset.ts - Updates render method to accommodate classes
second-gen/packages/swc/components/asset/asset.css - Adds CSS from spectrum-css
second-gen/packages/swc/components/asset/stories/asset.stories.ts - Adds comprehensive story coverage

Related issue(s)

SWC-1261

Screenshots (if appropriate)

Author's checklist

I have read the CONTRIBUTING and PULL_REQUESTS documents.
I have reviewed at the Accessibility Practices for this feature, see: Aria Practices
I have added automated tests to cover my changes.
I have included a well-written changeset if my change needs to be published.
I have included updated documentation if my change required it.

Reviewer's checklist

Includes a Github Issue with appropriate flag or Jira ticket number without a link
Includes thoughtfully written changeset if changes suggested include patch, minor, or major features (Note: I did not add a changeset, I wasn't sure that this was set up for barebones and didn't see any for progress circle or badge, but I'm happy to add one if I'm mistaken!)
Automated tests cover all use cases and follow best practices for writing
Validated on all supported browsers
All VRTs are approved before the author can update Golden Hash

Manual review test cases

Run second-gen Storybook:

yarn workspace @swc/components storybook

Storybook functionality:

All controls work correctly on Default story

Documentation:

All stories render correctly on Docs page
Story descriptions are clear and informative

First-gen verification (use the PR preview or run locally):

yarn start
# or
yarn docs:start

First-gen Asset remains unchanged and functional

Code review checklist:

TypeScript types are correct and properly exported
CSS follows second-gen patterns
No linter errors
Proper separation of concerns between core and SWC packages

Device review

Did it pass in Desktop?
Did it pass in (emulated) Mobile?
Did it pass in (emulated) iPad?

changeset-bot · 2025-11-06T09:02:03Z

⚠️ No Changeset found

Latest commit: 5f4f64d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

package.json

Rajdeepc

Good start!

Rajdeepc · 2025-11-11T06:54:20Z

2nd-gen/packages/swc/components/asset/asset.css

 }

-::slotted(*) {
+.spectrum-Asset-image {


This is a breaking change for the existing behaviour if consumers are slotting an <img> without class.
You can keep this class with a migration shim.

::slotted() { /* same rules */ } .spectrum-Asset-image { /* same rules */ }

but can we not break this for spectrum-two?

we should try not to treat S2 as a chance to introduce breaking changes unless they’re really necessary. These can have a big impact on adoption for product teams, so it’s worth being mindful about when and why we make them. If a change truly adds more value than the potential disruption, we can definitely go for it

2nd-gen/packages/swc/components/asset/asset.css

2nd-gen/packages/swc/components/asset/Asset.ts

Rajdeepc · 2025-11-11T07:01:27Z

2nd-gen/packages/swc/components/asset/asset.css

-.fileBackground {
-    fill: var(--spectrum-asset-file-background);
+.spectrum-Asset-folderBackground {
+    fill: var(--spectrum-gray-200);


Do these map to the same value? Can you surface up these changes in the PR documentation too for quick reference by reviewers. Its hard to visualize from here

These should be the same values, yes (and they look to be mapped correctly).

Before Cassondra left, she started removing some of the complexity from a chunk of components, asset being one of them. You can see the changes (and remove of the --spectrum-asset- variables) here: https://github.com/adobe/spectrum-css/pull/4257/files#diff-3a87a9f21648ac03bf4532993e3cdf48604b9862efd1c0ea68c3f6c2d44b32b3

In a different PR (that does the same and removes mods and some of the --spectrum prefixed variables for asset list and miller), Cassondra left her reasoning as to why: adobe/spectrum-css#4260 (comment)

Hopefully that's useful! I absolutely agree with @Rajdeepc that we should surface the changes in a changeset. None of these migrations have had a changeset yet. (I asked a similar question in Slack when I was working on status light). Do we have the changeset management situation figured out? I've been out, so it might be and we need a changeset!

marissahuysentruyt

I don't think I have any true changes to request! I did leave some additional context in response to a few of Rajdeep's comments, though- hopefully they're helpful!

2nd-gen/packages/swc/components/asset/Asset.ts

2nd-gen/packages/core/components/asset/Asset.base.ts

marissahuysentruyt · 2025-11-11T17:59:17Z

2nd-gen/packages/swc/components/asset/asset.css

-.fileBackground {
-    fill: var(--spectrum-asset-file-background);
+.spectrum-Asset-folderBackground {
+    fill: var(--spectrum-gray-200);


These should be the same values, yes (and they look to be mapped correctly).

Before Cassondra left, she started removing some of the complexity from a chunk of components, asset being one of them. You can see the changes (and remove of the --spectrum-asset- variables) here: https://github.com/adobe/spectrum-css/pull/4257/files#diff-3a87a9f21648ac03bf4532993e3cdf48604b9862efd1c0ea68c3f6c2d44b32b3

In a different PR (that does the same and removes mods and some of the --spectrum prefixed variables for asset list and miller), Cassondra left her reasoning as to why: adobe/spectrum-css#4260 (comment)

Hopefully that's useful! I absolutely agree with @Rajdeepc that we should surface the changes in a changeset. None of these migrations have had a changeset yet. (I asked a similar question in Slack when I was working on status light). Do we have the changeset management situation figured out? I've been out, so it might be and we need a changeset!

2nd-gen/packages/swc/components/asset/asset.css

package.json

marissahuysentruyt · 2025-11-12T18:03:47Z

@TarunAdobe I had one more thing!

Looking at the title of this PR, I'm curious if this work is a feat as opposed to a chore, since it's the migration into second-gen. Should we update this PR title to reflect that? Is my feat vs. chore logic even accurate? Asset is such a small component so I'm not sure if it's really a "breaking" change or not.

marissahuysentruyt

I went through it more in depth, and I have a few more questions for you. It still looks really great!

I'd like 2 changes I think:

I think the big thing for me ended up being in the stories file, just keeping the render method in the meta to align with how the other migrated components have it set up currently.
Pretty sure we need a :host selector in the CSS that also has the display: flex property.

2nd-gen/packages/swc/components/asset/asset.css

2nd-gen/packages/core/components/asset/Asset.base.ts

2nd-gen/packages/swc/components/asset/Asset.ts

2nd-gen/packages/swc/components/asset/stories/asset.stories.ts

Co-authored-by: Marissa Huysentruyt <[email protected]>

github-actions · 2025-11-18T07:53:46Z

📚 Branch Preview Links

🔍 First Generation 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:

Deployed to Azure Blob Storage: pr-5858

If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.

marissahuysentruyt

looks wonderful! I also verified that we get the console warnings in the browser:

I wanted to note that I'm actually seeing the alt text render for the default asset variant, with the image, right now. I believe that's because cloudflare is down this morning, though. We should make sure the image loads once cloudflare is back up.

TarunAdobe force-pushed the ttomar/asset-migration branch 3 times, most recently from 8500305 to e550a7e Compare November 6, 2025 11:07

TarunAdobe commented Nov 6, 2025

View reviewed changes

package.json Outdated Show resolved Hide resolved

chore: migrate asset

58474d7

TarunAdobe force-pushed the ttomar/asset-migration branch from e550a7e to 58474d7 Compare November 6, 2025 11:13

TarunAdobe marked this pull request as ready for review November 6, 2025 13:54

TarunAdobe requested a review from a team as a code owner November 6, 2025 13:54

Rajdeepc requested changes Nov 11, 2025

View reviewed changes

Rajdeepc added the Status: Ready for review PR ready for review or re-review. label Nov 11, 2025

marissahuysentruyt reviewed Nov 11, 2025

View reviewed changes

TarunAdobe added 2 commits November 13, 2025 16:58

chore: review suggestions

b5ba2d3

chore: merge main:

1b8d3f0

TarunAdobe changed the title ~~chore: migrate asset~~ feat: migrate asset to s2 styles and second-gen architecture Nov 13, 2025

TarunAdobe requested review from Rajdeepc, caseyisonit and marissahuysentruyt November 13, 2025 11:50

marissahuysentruyt requested changes Nov 13, 2025

View reviewed changes

TarunAdobe and others added 2 commits November 17, 2025 13:53

Merge branch 'main' into ttomar/asset-migration

a3ae158

chore: update asset css

527b0fc

Co-authored-by: Marissa Huysentruyt <[email protected]>

TarunAdobe force-pushed the ttomar/asset-migration branch from 2d7427e to 527b0fc Compare November 17, 2025 08:33

chore: review changes

9b12ba6

TarunAdobe requested a review from marissahuysentruyt November 17, 2025 09:43

adobe deleted a comment from github-actions bot Nov 17, 2025

Merge branch 'main' into ttomar/asset-migration

9dd81b4

marissahuysentruyt approved these changes Nov 18, 2025

View reviewed changes

Merge branch 'main' into ttomar/asset-migration

3c3f03e

TarunAdobe added 2 commits November 21, 2025 14:53

Merge branch 'main' into ttomar/asset-migration

7253110

Merge branch 'main' into ttomar/asset-migration

6ee03cc

Rajdeepc approved these changes Nov 21, 2025

View reviewed changes

Merge branch 'main' into ttomar/asset-migration

5f4f64d

feat: migrate asset to s2 styles and second-gen architecture #5858

Are you sure you want to change the base?

feat: migrate asset to s2 styles and second-gen architecture #5858

Uh oh!

Conversation

TarunAdobe commented Nov 6, 2025 • edited by marissahuysentruyt Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and context

What changed

Related issue(s)

Screenshots (if appropriate)

Author's checklist

Reviewer's checklist

Manual review test cases

Device review

Uh oh!

changeset-bot bot commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

Uh oh!

Rajdeepc left a comment

Choose a reason for hiding this comment

Uh oh!

Rajdeepc Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TarunAdobe Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Rajdeepc Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Rajdeepc Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

marissahuysentruyt Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marissahuysentruyt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marissahuysentruyt Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

marissahuysentruyt commented Nov 12, 2025

Uh oh!

marissahuysentruyt left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📚 Branch Preview Links

🔍 First Generation Visual Regression Test Results

Uh oh!

marissahuysentruyt left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

TarunAdobe commented Nov 6, 2025 •

edited by marissahuysentruyt

Loading

changeset-bot bot commented Nov 6, 2025 •

edited

Loading

Rajdeepc Nov 11, 2025 •

edited

Loading

marissahuysentruyt Nov 11, 2025 •

edited

Loading

marissahuysentruyt Nov 11, 2025 •

edited

Loading

marissahuysentruyt left a comment •

edited

Loading

github-actions bot commented Nov 18, 2025 •

edited

Loading