Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(thumbnail): S2 migration #3367

Merged
merged 2 commits into from
Nov 12, 2024
Merged

feat(thumbnail): S2 migration #3367

merged 2 commits into from
Nov 12, 2024

Conversation

cdransf
Copy link
Member

@cdransf cdransf commented Nov 5, 2024 •
edited
Loading

Description

For CSS-1023.

Replaces corner-radius-75 with thumbnail-corner-radius.

Remaining changes in the design specifications pertain to the underlying checkerboard component.

Migrations for components leveraging thumbnail such as treeview and table will be handled in separate PRs.

Validation steps

  1. Access the Storybook URL for the PR (or fetch the branch and run it locally)
  2. Navigate to the thumbnail story
  3. Verify that there are no visible regressions.

Note: the scope of this migration was quite small and was limited to an updated token alias per the Figma docs.

To-do list

  • I have read the contribution guidelines.
  • I have updated relevant storybook stories and templates.
  • I have tested these changes in Windows High Contrast mode.
  • If my change impacts other components, I have tested to make sure they don't break.
  • If my change impacts documentation, I have updated the documentation accordingly.
  • ✨ This pull request is ready to merge. ✨

@cdransf cdransf added wip This is a work in progress, don't judge. skip_vrt Add to a PR to skip running VRT (but still pass the action) labels Nov 5, 2024
@cdransf cdransf self-assigned this Nov 5, 2024
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Nov 5, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: ef77162

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@spectrum-css/thumbnail Minor
@spectrum-css/table Major
@spectrum-css/treeview Major

Not sure what this means? Click here to learn what changesets are.

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

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 5, 2024 •
edited
Loading

🚀 Deployed on https://pr-3367--spectrum-css.netlify.app

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 5, 2024 •
edited
Loading

File metrics

Summary

Total size: 4.29 MB*
Total change (Δ): ⬇ 2.05 KB (-0.05%)

Table reports on changes to a package's main file. Other changes can be found in the collapsed Details section below.

Package Size Δ
thumbnail 7.67 KB ⬇ 0.72 KB

Details

thumbnail

File Head Base Δ
index-base.css 7.67 KB 8.38 KB ⬇ 0.72 KB (-8.39%)
index-vars.css 7.67 KB 8.38 KB ⬇ 0.72 KB (-8.39%)
index.css 7.67 KB 8.38 KB ⬇ 0.72 KB (-8.39%)
metadata.json 3.55 KB 3.48 KB ⬆ 0.06 KB (1.77%)
* Size determined by adding together the size of the main file for all packages in the library.
* Results are not gzipped or minified.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

@cdransf cdransf marked this pull request as ready for review November 5, 2024 18:10
@cdransf cdransf added size-1 XS ~1-6hrs; nearly trivial, a few hours, could do more than one in a single day. ready-for-review and removed skip_vrt Add to a PR to skip running VRT (but still pass the action) labels Nov 5, 2024
@cdransf cdransf changed the title chore(thumbnail): update corner-radius token chore(thumbnail): migrate to S2 Nov 5, 2024
@cdransf cdransf force-pushed the cdransf/s2-thumbnail-migration branch from 7b3c26f to 9108ccc Compare November 5, 2024 19:15
@cdransf cdransf changed the title chore(thumbnail): migrate to S2 feat(thumbnail): migrate to S2 Nov 5, 2024
@cdransf cdransf changed the title feat(thumbnail): migrate to S2 feat(thumbnail): migrate s2 link Nov 5, 2024
@cdransf cdransf changed the title feat(thumbnail): migrate s2 link feat(thumbnail): migrate s2 thumbnail Nov 5, 2024
marissahuysentruyt
marissahuysentruyt reviewed Nov 7, 2024
View reviewed changes
Copy link
Collaborator

@marissahuysentruyt marissahuysentruyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work on this. I left a few remaining questions I had for you, before approving. I also checked out treeview variant with thumbnails, as well as the table thumbnail variants, and I think they look alright. 👍 I assume any further styling of those thumbnails would come during the treeview and table anyways. (although I did have a question about the thumbnail layer note)

components/thumbnail/index.css Outdated Show resolved Hide resolved
components/thumbnail/index.css Show resolved Hide resolved
components/thumbnail/index.css Show resolved Hide resolved
@cdransf cdransf force-pushed the cdransf/s2-thumbnail-migration branch from 9108ccc to 1d3facd Compare November 7, 2024 20:05
@cdransf cdransf force-pushed the cdransf/s2-thumbnail-migration branch from 1d3facd to a0a75ec Compare November 11, 2024 16:02
@cdransf cdransf changed the title feat(thumbnail): migrate s2 thumbnail feat(thumbnail): S2 migration Nov 11, 2024
@cdransf cdransf force-pushed the cdransf/s2-thumbnail-migration branch from a0a75ec to 273bc2e Compare November 12, 2024 19:27
marissahuysentruyt
marissahuysentruyt approved these changes Nov 12, 2024
View reviewed changes
Copy link
Collaborator

@marissahuysentruyt marissahuysentruyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great! Happy we figured out a good way to handle that opacity checkerboard token!

Thanks for looking into that treeview question I had. I think it's appropriate we take care of the "layer" thumbnail stuff along with treeview.

@cdransf cdransf merged commit 2c62239 into spectrum-two Nov 12, 2024
11 checks passed
@cdransf cdransf deleted the cdransf/s2-thumbnail-migration branch November 12, 2024 21:27
@github-actions github-actions bot mentioned this pull request Nov 12, 2024
Merged
castastrophe pushed a commit that referenced this pull request Dec 27, 2024
@cdransf @castastrophe
feat(thumbnail): S2 migration (#3367)
5f6f035 
* feat(thumbnail): migrate to S2

* feat(thumbnail): add customstyles to scope change to thumbnail
castastrophe pushed a commit that referenced this pull request Dec 29, 2024
@cdransf @castastrophe
feat(thumbnail): S2 migration (#3367)
81adc7e 
* feat(thumbnail): migrate to S2

* feat(thumbnail): add customstyles to scope change to thumbnail
castastrophe pushed a commit that referenced this pull request Dec 29, 2024
@cdransf @castastrophe
feat(thumbnail): S2 migration (#3367)
1d8872e 
* feat(thumbnail): migrate to S2

* feat(thumbnail): add customstyles to scope change to thumbnail
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marissahuysentruyt marissahuysentruyt marissahuysentruyt approved these changes

@jawinn jawinn Awaiting requested review from jawinn

@rise-erpelding rise-erpelding Awaiting requested review from rise-erpelding

@pfulton pfulton Awaiting requested review from pfulton

@castastrophe castastrophe Awaiting requested review from castastrophe

Assignees

@cdransf cdransf

Labels
ready-for-review size-1 XS ~1-6hrs; nearly trivial, a few hours, could do more than one in a single day. wip This is a work in progress, don't judge.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cdransf @marissahuysentruyt