chore: migrate asset #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: 58474d7

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

github-actions · 2025-11-06T09:04:01Z

📚 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:

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.

TarunAdobe · 2025-11-06T11:07:57Z

package.json

        "start": "yarn start:1st-gen & yarn start:2nd-gen",
        "start:1st-gen": "yarn workspace @spectrum-web-components/1st-gen start",
-        "start:2nd-gen": "yarn workspace @adobe/swc start",
+        "start:2nd-gen": "yarn workspace @adobe/swc storybook",


There is no yarn start command in 2nd gen swc package

Is this the start command you were looking for? Or does this do something else?

spectrum-web-components/2nd-gen/package.json

Line 24 in 9a93654

"start": "run-p dev:core dev:analyze storybook",

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 */ }

Rajdeepc · 2025-11-11T06:58:31Z

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

-    }
+.spectrum-Asset-fileOutline {
+    fill: var(--spectrum-gray-500);
 }


Is this intentional? High-contrast users may get incorrect colors or no contrast adjustments.

This was removed in the same PR I mentioned above, where we were removing the mods for asset. Personally, I like this removal after talking with @5t3ph during the app frame side nav WHCM stuff. I don't think this currentcolor is necessary and just adds to the CSS we're shipping for no super strong reason. We can rely on the default handling of high contrast instead of defining this icon background color.

This is with the high contrast variable. I don't think anything is really being added with this query, but I could be wrong.

Thanks for the explanation. Looks fine.

Rajdeepc · 2025-11-11T06:59:52Z

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

+                class=${classMap({
+                    ['spectrum-Asset']: true,
+                })}


only one static class is set you could just set class="spectrum-Asset" directly. minor simplification.

i would like to keep the classMap pattern so its easily extensible if it arises... not a deal breaker though

ideally there is no performance impact here since it just adds a tiny internal Set to render the class list but yea I’m fine either way unless there’s a strong preference to simplify.

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!

Please surface up a changeset on this, we will check how the tool is scanning it.

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!

marissahuysentruyt · 2025-11-11T17:32:00Z

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

+ *
 * @element swc-asset
- * @slot - content to be displayed in the asset when an acceptable value for `file` is not present
+ * @slot - content to be displayed when no `variant` is set (typically an <img>)


Right now, these docs are missing the <img> tag in the parentheses) in the description in the control table:

just wrapping it in backticks seems to do the trick.

Suggested change

* @slot - content to be displayed when no `variant` is set (typically an <img>)

* @slot - The content to be displayed when no `variant` is set (typically an `<img>`)

marissahuysentruyt · 2025-11-11T17:35:51Z

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

+ * Use an asset element to visually represent a file, folder, or image.
+ * File and folder representations center themselves within the available space.
+ * Images are contained to the element’s size and centered.


This feels like maybe something we would want to display on the docs page. What do you think about moving this JSDoc to the stories file instead, so that it displays under the "Asset" title?

This is something I stole from the divider stories file. 😊

marissahuysentruyt · 2025-11-11T17:39:44Z

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

+    public variant: AssetVariant | undefined;

+    /**
+     * Accessible label for the asset’s SVG variant.


Super nit pick: our variants are technically named them "file" or "folder," and not "SVG." Is it odd or confusing to mention an SVG variant? I'll leave it up to you if you want to change it.

Suggested change

* Accessible label for the asset’s SVG variant.

* Accessible label for the asset’s file and folder variants.

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!

marissahuysentruyt · 2025-11-11T20:25:37Z

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

-    }
+.spectrum-Asset-fileOutline {
+    fill: var(--spectrum-gray-500);
 }


This was removed in the same PR I mentioned above, where we were removing the mods for asset. Personally, I like this removal after talking with @5t3ph during the app frame side nav WHCM stuff. I don't think this currentcolor is necessary and just adds to the CSS we're shipping for no super strong reason. We can rely on the default handling of high contrast instead of defining this icon background color.

This is with the high contrast variable. I don't think anything is really being added with this query, but I could be wrong.

marissahuysentruyt · 2025-11-11T20:27:45Z

package.json

        "start": "yarn start:1st-gen & yarn start:2nd-gen",
        "start:1st-gen": "yarn workspace @spectrum-web-components/1st-gen start",
-        "start:2nd-gen": "yarn workspace @adobe/swc start",
+        "start:2nd-gen": "yarn workspace @adobe/swc storybook",


Is this the start command you were looking for? Or does this do something else?

spectrum-web-components/2nd-gen/package.json

Line 24 in 9a93654

"start": "run-p dev:core dev:analyze storybook",

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

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

	* @slot - content to be displayed when no `variant` is set (typically an <img>)
	* @slot - The content to be displayed when no `variant` is set (typically an `<img>`)

	* Accessible label for the asset’s SVG variant.
	* Accessible label for the asset’s file and folder variants.

chore: migrate asset #5858

Are you sure you want to change the base?

chore: migrate asset #5858

Uh oh!

Conversation

TarunAdobe commented Nov 6, 2025 • edited 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!

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

Uh oh!

📚 Branch Preview

🔍 Visual Regression Test Results

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

marissahuysentruyt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

TarunAdobe commented Nov 6, 2025 •

edited

Loading

changeset-bot bot commented Nov 6, 2025 •

edited

Loading

github-actions 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