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.

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

Feat(design-tokens): Introduce structure of exported tokens to JS #1101

Merged
merged 15 commits into from
Nov 14, 2023

Conversation

literat
Copy link
Collaborator

@literat literat commented Oct 19, 2023

Description

Additional context

Issue reference


Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Follow the PR Title/Commit Message Convention.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@netlify
Copy link

netlify bot commented Oct 19, 2023

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

Name Link
🔨 Latest commit fabd72d
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system-storybook/deploys/65536007588dd3000829c2bb

@github-actions github-actions bot added the feature New feature or request label Oct 19, 2023
@netlify
Copy link

netlify bot commented Oct 19, 2023

Deploy Preview for spirit-design-system-demo ready!

Name Link
🔨 Latest commit fabd72d
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system-demo/deploys/655360072ee435000837ac04
😎 Deploy Preview https://deploy-preview-1101--spirit-design-system-demo.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.

@netlify
Copy link

netlify bot commented Oct 19, 2023

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

Name Link
🔨 Latest commit fabd72d
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system-validations/deploys/65536007114331000974fbdc

@netlify
Copy link

netlify bot commented Oct 19, 2023

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

Name Link
🔨 Latest commit fabd72d
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system-react/deploys/655360072b59f90008419ea2

@nx-cloud
Copy link

nx-cloud bot commented Oct 19, 2023

☁️ Nx Cloud Report

CI is running/has finished running commands for commit cfcc21c. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 4 targets

Sent with 💌 from NxCloud.

@github-actions
Copy link
Contributor

Coverage Status

coverage: 69.754% (-26.7%) from 96.46% when pulling 037396b on feat/DS-1012-js-design-tokens into 0cb30d5 on main.

@literat literat marked this pull request as ready for review October 20, 2023 10:47
@literat literat force-pushed the feat/DS-1012-js-design-tokens branch 2 times, most recently from d50498c to 1d3bf3d Compare October 20, 2023 13:14
@literat literat force-pushed the feat/DS-1012-js-design-tokens branch from 9422b0c to 4c49c0c Compare November 3, 2023 13:35
@coveralls
Copy link

coveralls commented Nov 3, 2023

Coverage Status

coverage: 80.121% (-16.3%) from 96.46%
when pulling fabd72d on feat/DS-1012-js-design-tokens
into 5da46d2 on main.

@literat literat force-pushed the feat/DS-1012-js-design-tokens branch from a00dbd0 to 50cb350 Compare November 3, 2023 16:31
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.

I feel like this is several PRs merged into one. It is quite hard to navigate through this PR, especially without any README. I assume we need some at least for the Offcanvas plugin, because now only you know how to set the breakpoint. So please document this better.

Also, I am a bit afraid if it won't break anything, as you modify a lot of files and packages, but hopefully it won't. Also I didn't find the CSS variable implementation?

packages/web/src/js/AutoResize.ts Outdated Show resolved Hide resolved
packages/web/src/js/BaseComponent.ts Outdated Show resolved Hide resolved
packages/web/src/js/Offcanvas.ts Outdated Show resolved Hide resolved
packages/web/src/js/Offcanvas.ts Outdated Show resolved Hide resolved
packages/web/src/js/Offcanvas.ts Show resolved Hide resolved
packages/web/src/js/dom/__tests__/Manipulator.test.ts Outdated Show resolved Hide resolved
packages/web/src/js/utils/Config.ts Show resolved Hide resolved
packages/web/src/js/utils/__tests__/Config.test.ts Outdated Show resolved Hide resolved
packages/web/src/js/utils/__tests__/Config.test.ts Outdated Show resolved Hide resolved
packages/web/src/js/utils/__tests__/Config.test.ts Outdated Show resolved Hide resolved
@literat literat force-pushed the feat/DS-1012-js-design-tokens branch from 7fa5fdc to f8c4f9a Compare November 6, 2023 13:29
@literat
Copy link
Collaborator Author

literat commented Nov 6, 2023

I feel like this is several PRs merged into one. It is quite hard to navigate through this PR, especially without any README. I assume we need some at least for the Offcanvas plugin, because now only you know how to set the breakpoint. So please document this better.

Try to go commit-by-commit. The build process is a little bit complicated because design tokens must be built first.

Also, I am a bit afraid if it won't break anything, as you modify a lot of files and packages, but hopefully it won't. Also I didn't find the CSS variable implementation?

This was a problem with the rebase I have made. The CSS variable was added by me and also I have introduced the Offcanvas Readme.

@literat literat requested a review from crishpeen November 6, 2023 13:43
@literat literat force-pushed the feat/DS-1012-js-design-tokens branch from 690ba33 to bd577b0 Compare November 6, 2023 15:35
packages/web/src/js/Modal.ts Show resolved Hide resolved
packages/web/src/js/Offcanvas.ts Outdated Show resolved Hide resolved
packages/web/src/scss/components/Offcanvas/README.md Outdated Show resolved Hide resolved
packages/web/src/scss/components/Offcanvas/README.md Outdated Show resolved Hide resolved
packages/web/src/scss/components/Offcanvas/README.md Outdated Show resolved Hide resolved
packages/web/src/scss/components/Offcanvas/README.md Outdated Show resolved Hide resolved
packages/web/src/scss/components/Offcanvas/README.md Outdated Show resolved Hide resolved
packages/web/src/scss/components/Offcanvas/README.md Outdated Show resolved Hide resolved
@literat literat force-pushed the feat/DS-1012-js-design-tokens branch from c61bf50 to e96c830 Compare November 10, 2023 09:00
@literat literat requested a review from crishpeen November 10, 2023 09:29
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.

few small fixes and it is hopefully ready to merge

@crishpeen crishpeen self-requested a review November 14, 2023 09:51
@literat literat force-pushed the feat/DS-1012-js-design-tokens branch from 70b4890 to fabd72d Compare November 14, 2023 11:54
@literat literat merged commit 87af0cd into main Nov 14, 2023
25 checks passed
@literat literat deleted the feat/DS-1012-js-design-tokens branch November 14, 2023 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants