Skip to content

Feat(exporter): Add new files for device tokens #2056

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.

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

Merged
merged 2 commits into from
May 7, 2025

Conversation

pavelklibani
Copy link
Contributor

Description

Additional context

Issue reference

Form | Radius | New design-tokens structure

@pavelklibani pavelklibani self-assigned this May 5, 2025
@github-actions github-actions bot added the feature New feature or request label May 5, 2025
Copy link

netlify bot commented May 5, 2025

Deploy Preview for spirit-design-system-storybook canceled.

Name Link
🔨 Latest commit cf85c0b
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system-storybook/deploys/681b4caf96f3490008912771

Copy link

netlify bot commented May 5, 2025

Deploy Preview for spirit-design-system ready!

Name Link
🔨 Latest commit cf85c0b
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system/deploys/681b4caf20eee50008d3d14e
😎 Deploy Preview https://deploy-preview-2056--spirit-design-system.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@pavelklibani pavelklibani force-pushed the feat/ds-1872-form-radius-design-tokens branch from 92fe8fc to 2abb545 Compare May 5, 2025 07:11
@pavelklibani pavelklibani marked this pull request as ready for review May 5, 2025 08:05
@pavelklibani pavelklibani marked this pull request as draft May 5, 2025 08:08
@pavelklibani pavelklibani force-pushed the feat/ds-1872-form-radius-design-tokens branch from 2abb545 to 71e805d Compare May 5, 2025 08:23
@pavelklibani pavelklibani marked this pull request as ready for review May 5, 2025 08:23
Copy link
Member

@crishpeen crishpeen left a comment

Choose a reason for hiding this comment

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

Please change the commit scope. It should be design-tokens

Copy link
Member

@crishpeen crishpeen left a comment

Choose a reason for hiding this comment

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

LGTM, please add e2e visual tests to check nothing breaks. And please change the commit scope.

@crishpeen
Copy link
Member

Also, please let Jirka know what we cannot move to the Device collection now (because of the map/object conflict).

@literat
Copy link
Collaborator

literat commented May 6, 2025

I am sorry, but the naming does not make sense to me. Why device? I will expect there tv, mobile, desktop and other devices; however, what I see are buttons, forms, etc...

Wouldn't be better to use responsivity tokens or something else?

@pavelklibani pavelklibani added the run-visual-tests Runs visual regression testing on this PR label May 6, 2025
@pavelklibani pavelklibani force-pushed the feat/ds-1872-form-radius-design-tokens branch from fd0b804 to 15a2a9d Compare May 6, 2025 16:42
Copy link
Contributor

github-actions bot commented May 6, 2025

@crishpeen
Copy link
Member

I am sorry, but the naming does not make sense to me. Why device? I will expect there tv, mobile, desktop and other devices; however, what I see are buttons, forms, etc...

Wouldn't be better to use responsivity tokens or something else?

This is based on the usage in Figma, where you choose Appearance.

image

We came up with this name, because it is best for designers and ok for us. In spirit-web you won't even know the directory, because as you can see, it is exported from @tokens too. So you will still use just border-radius: tokens.$button-small-radius-mobile; like with all other tokens.

@pavelklibani pavelklibani added run-visual-tests Runs visual regression testing on this PR and removed run-visual-tests Runs visual regression testing on this PR labels May 7, 2025
Copy link
Contributor

github-actions bot commented May 7, 2025

@pavelklibani pavelklibani force-pushed the feat/ds-1872-form-radius-design-tokens branch from 15a2a9d to 43dfd18 Compare May 7, 2025 10:38
@pavelklibani pavelklibani force-pushed the feat/ds-1872-form-radius-design-tokens branch from 43dfd18 to cf85c0b Compare May 7, 2025 12:06
@pavelklibani
Copy link
Contributor Author

I am sorry, but the naming does not make sense to me. Why device? I will expect there tv, mobile, desktop and other devices; however, what I see are buttons, forms, etc...
Wouldn't be better to use responsivity tokens or something else?

This is based on the usage in Figma, where you choose Appearance.

image We came up with this name, because it is best for designers and ok for us. In spirit-web you won't even know the directory, because as you can see, it is exported from @tokens too. So you will still use just `border-radius: tokens.$button-small-radius-mobile;` like with all other tokens.

It was discussed in the office, and we decided to keep it named devices.

@pavelklibani pavelklibani merged commit d8a9cb2 into main May 7, 2025
23 checks passed
@pavelklibani pavelklibani deleted the feat/ds-1872-form-radius-design-tokens branch May 7, 2025 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request run-visual-tests Runs visual regression testing on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants