Chore/2873 remove routing from token setup separate deprecated project - #3482
Draft
Diaan wants to merge 44 commits into
Draft
Chore/2873 remove routing from token setup separate deprecated project#3482Diaan wants to merge 44 commits into
Diaan wants to merge 44 commits into
Conversation
…adability and maintainability
- Updated `build-themes.js` to include handling for deprecated themes. - Introduced a new function to create file configurations for deprecated themes. - Adjusted the theme filtering logic to accommodate deprecated tokens. - Added a new workspace for `deprecated-tokens` with its own build scripts. - Created `.tokensstudio.json` for deprecated tokens configuration. - Updated `package.json` files to include scripts for importing deprecated tokens. - Modified the main `package.json` to include the new workspace and import script.
…eparate-deprecated-project
…recated-project' of https://github.com/sl-design-system/components into chore/2873-remove-routing-from-token-setup-separate-deprecated-project
…ting-from-token-setup-separate-deprecated-project
… removed redundant code blocks and optimized existing functions.
…ting-from-token-setup-separate-deprecated-project
- Created developing.css with typography settings for the "developing" user group, including font sizes, line heights, letter spacing, and variable spaces. - Created early.css with typography settings for the "early" user group, mirroring the structure of developing.css but with adjusted values for font sizes and spacing.
|
Contributor
🕸 Preview deploys |
- Created developing.css with typography variables for the "developing" user group. - Created early.css with typography variables for the "early" user group. - Defined font sizes, line heights, letter spacing, and other related styles for both user groups.
…ting-from-token-setup-separate-deprecated-project
…configurations - Introduced new JSON files for scale, space, and typography tokens to enhance design consistency. - Updated the paths in the package.json for token imports to reflect new directory structure. - Modified metadata files to ensure proper formatting and consistency across legacy themes. - Adjusted studio.lock to reflect changes in source references and updated timestamps.
- Updated the "import-tokens" script in package.json to only execute the "studio pull" command. - Modified the setup-themes.js script to read multiple typography CSS files and combine their contents into a single output file, improving the typography setup process. - Updated the studio.lock file with new generated timestamps and hashes for dependencies, ensuring consistency in the build process.
Comment on lines
+14
to
+23
| const typographyCssFiles = [ | ||
| './export/core-css/Device/desktop.css', | ||
| './export/core-css/Device/mobile.css', | ||
| './export/core-css/Device/tablet.css', | ||
| './export/core-css/User-Group/advanced.css', | ||
| './export/core-css/User-Group/developing.css', | ||
| './export/core-css/User-Group/early.css', | ||
| '../packages/themes/core/typography.css', | ||
| `${theme}/typography.css` | ||
| ]; |
Comment on lines
+35
to
+42
| Promise.all(promises) | ||
| .then(parts => { | ||
| writeFile(destinationTypography, parts.join('\n\n'), err => { | ||
| if (err) console.error(`Error writing ${theme} typography:`, err); | ||
| else console.log(`${theme} typography done`); | ||
| }); | ||
| }) | ||
| .catch(err => console.error(`Error reading typography files for ${theme}:`, err)); |
Comment on lines
+200
to
+220
| filter: token => | ||
| token.$type === 'color' && | ||
| (token.original?.$value?.startsWith('rgba') || token.original?.$value?.startsWith('set_alpha')), | ||
| transform: token => { | ||
| const originalValue = token.original?.$value; | ||
| const [_, color, opacity] = originalValue.startsWith('rgba') | ||
| ? (originalValue.match(/rgba\(\s*(\S+)\s*,\s*(\S+)\)/) ?? []) | ||
| : originalValue.startsWith('set_alpha') | ||
| ? (originalValue.match(/set_alpha\(\s*(\S+)\s*,\s*(\S+)\)/) ?? []) | ||
| : []; | ||
|
|
||
| if (color && opacity) { | ||
| if (opacity.endsWith('%')) { | ||
| token.original.$value = `color-mix(in srgb, ${color} ${opacity}, transparent)`; | ||
| } else { | ||
| token.original.$value = `color-mix(in srgb, ${color} calc(${opacity} * 100%), transparent)`; | ||
| } | ||
| } | ||
|
|
||
| return token.$value; | ||
| } |
Comment on lines
7
to
+10
| "ref": { | ||
| "type": "branch", | ||
| "name": "main" | ||
| }, | ||
| "output": "../packages/tokens/src/tokens", | ||
| "format": "raw" | ||
| "name": "typography-add" | ||
| } |
| else console.log(`${theme} typography done`); | ||
| }); | ||
| }) | ||
| .catch(err => console.error(`Error reading typography files for ${theme}:`, err)); | ||
| }; | ||
|
|
||
| const setupAllThemes = async () => { |
Comment on lines
+28
to
30
| "output": "../packages/tokens/src/typography-sizing-spacing", | ||
| "format": "css" | ||
| } |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 58 out of 219 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
scripts/build-themes.js:221
- The
sl/color/transparentColorMixtransform is redundant with the existingconvert-set-alpha-to-color-mixpreprocessor and is currently not included in the platformtransformslist. Keeping an unused (and partially overlapping) transform increases maintenance cost and can cause confusion about which conversion is authoritative.
| cp(source, destination, () => console.log(`${theme} done`)); | ||
| const sourceGlobal = join(cwd, '../packages/themes/core/global.css'); | ||
| const destinationGlobal = join(cwd, `${theme}/global.css`); | ||
| cp(sourceGlobal, destinationGlobal, () => console.log(`🌍 ✅ ${theme}`)); |
…ting-from-token-setup-separate-deprecated-project
…consistent newline endings - Updated scope definitions in tablet.json, space.json, typography.json, and scale.json to use array formatting for single items. - Ensured all JSON files in the slds-legacy directory have consistent newline endings. - Updated studio.lock with new generated timestamps and updated hashes.
…tent structure across device, typography, and space tokens. Added missing newlines at the end of several JSON files for compliance with formatting standards. Updated theme setup script to reorder CSS imports for User-Group styles. Updated studio.lock with new generated timestamps and hash values.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 57 out of 218 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
.storybook/stories/typography.stories.ts:146
- Leftover
console.loginparseCells()will spam the console and slow down Storybook when the grid is rendered.
console.log('specsContainer', specsContainer);
});
.storybook/stories/typography.stories.ts:150
variantis read once at module initialization time, so changing the Storybook “Target Group” toolbar won’t update the fallback markers/conditional rendering inTypographyStyles. This makes the story show the wrong states after switching user groups.
const variant = document.documentElement.getAttribute('data-user-group') ?? 'advanced';
scripts/build-themes.js:221
- The new
sl/color/transparentColorMixtransform currently mutatestoken.original.$valuebut returnstoken.$value, so it won’t change the generated output. Also, it assumes numeric alpha values are 0–1; the exported tokens in this PR include 0–255-style alphas (e.g. 229.5), which would become invalid percentages if converted asopacity * 100%.
Comment on lines
+9
to
+16
| .display { | ||
| font-size: var(--sl-display-md-font-size); | ||
| font-weight: var(--sl-typography-display-md-font-weight); | ||
| line-height: var(--sl-display-md-line-height); | ||
| letter-spacing: var(--sl-display-md-letter-spacing); | ||
|
|
||
| &.lg { | ||
| font-size: var(--sl-display-lg-font-size); |
Comment on lines
+10
to
+12
| import { Icon } from '../../packages/components/icon/src/icon'; | ||
| import { faPlanetRinged } from '@fortawesome/pro-regular-svg-icons'; | ||
| import { Popover } from '@sl-design-system/popover'; |
…eparate-deprecated-project
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 57 out of 218 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
.storybook/stories/typography.stories.ts:12
Popoveris imported but never used in this story, which will fail lint/type checks in many setups.
import { Popover } from '@sl-design-system/popover';
| cellSpecs.classList.add('text', 'sm'); | ||
| specsContainer?.appendChild(cellSpecs); | ||
| } | ||
| console.log('specsContainer', specsContainer); |
| }); | ||
| }); | ||
| }; | ||
| const variant = document.documentElement.getAttribute('data-user-group') ?? 'advanced'; |
Comment on lines
200
to
+206
| resources.global ??= document.head.appendChild(document.createElement('link')); | ||
| resources.global.href = `/themes/${themeId}/global.css`; | ||
| resources.global.rel = 'stylesheet'; | ||
|
|
||
| resources.typography ??= document.head.appendChild(document.createElement('link')); | ||
| resources.typography.href = `/themes/${themeId}/typography.css`; | ||
| resources.typography.rel = 'stylesheet'; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.