feat: create themes flag to build tokens #2357

dcoa · 2023-06-05T23:19:45Z

Description

This PR is a proposal to be able to build tokens of any custom theme throughout CLI. Also, add a function to create the index file for the output, understanding that the build-scss command needs this file.

  themes/ 
    light/
    │  ├─ index.css
    │  ├─ other_css_files
    dark/
    │  ├─ index.css
    │  ├─ other_css_files
    some_other_custom_theme/
    │  ├─ index.css
    │  ├─ other_css_files

The user can run something like build-design-tokens --build-dir ./<folder_path> --source /<folder_path> --themes custom_theme_a custom_theme_b light

Merge Checklist

If your update includes visual changes, have they been reviewed by a designer? Send them a link to the Netlify deploy preview, if applicable.
Does your change adhere to the documented style conventions?
Do any prop types have missing descriptions in the Props API tables in the documentation site (check deploy preview)?
Were your changes tested using all available themes (see theme switcher in the header of the deploy preview, under the "Settings" icon)?
Were your changes tested in the example app?
Is there adequate test coverage for your changes?
Consider whether this change needs to reviewed/QA'ed for accessibility (a11y). If so, please add wittjeff and adamstankiewicz as reviewers on this PR.

Post-merge Checklist

Verify your changes were released to NPM at the expected version.
If you'd like, share your contribution in #show-and-tell.
🎉 🙌 Celebrate! Thanks for your contribution.

@adamstankiewicz

openedx-webhooks · 2023-06-05T23:19:49Z

Thanks for the pull request, @dcoa! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

adamstankiewicz

Allowing users of the CLI to pass in custom themes makes a lot of sense! @edx/brand packages could begin defining their own themes before upstream Paragon does. The @edx/frontend-build and @edx/frontend-platform PRs should be able to support any custom themes as well already, if that is a concern.

adamstankiewicz · 2023-06-07T05:26:36Z

tokens/utils.js

@@ -162,7 +162,37 @@ async function transformInPath(location, variablesMap, transformType = 'definiti
  }
 }

+
+function createIndexFile(buildDir, isTheme, themeVariant){


I agree having it programmatically create the index file makes sense! 🥇

adamstankiewicz · 2023-06-07T05:27:14Z

tokens/build-tokens.js

+// This line creates the index file for core folder, specially when buildDir is outside Paragon.
+createIndexFile(buildDir,false);
+
+const THEME_VARIANTS = themes ? themes : ['light'];


nit:

Suggested change

const THEME_VARIANTS = themes ? themes : ['light'];

const THEME_VARIANTS = themes || ['light'];

adamstankiewicz · 2023-06-07T05:30:39Z

tokens/utils.js

@@ -162,7 +162,37 @@ async function transformInPath(location, variablesMap, transformType = 'definiti
  }
 }

+
+function createIndexFile(buildDir, isTheme, themeVariant){


nit: It may be worth changing the function signature here to be more like kwargs in python, e.g. function createIndexFile({ buildDir, isTheme, themeVariant }) {. Makes it more explicit what args you're passing in at first glance.

adamstankiewicz · 2023-06-07T05:34:45Z

tokens/build-tokens.js


 program
  .version('0.0.1')
  .description('CLI to build design tokens for various platforms (currently only CSS is supported) from Paragon Design Tokens.')
  .option('--build-dir <char>', 'A path to directory where to put files with built tokens, must end with a /.', './build/')
  .option('--source <char>', 'A path where to look for additional tokens that will get merged with Paragon ones, must be a path to root directory of the token files that contains "root" and "themes" subdirectories.')
  .option('--source-tokens-only', 'If provided, only tokens from --source will be included in the output; Paragon tokens will be used for references but not included in the output.')
+  .option('--themes <themes...>', 'A list of all the themes do you want to build, by default Paragn build light theme.')


nit grammar and typo. Perhaps something like:

Suggested change

.option('--themes <themes...>', 'A list of all the themes do you want to build, by default Paragn build light theme.')

.option('--themes <themes...>', 'A list of theme variants to build. By default, Paragon currently only supports a light theme.')

adamstankiewicz · 2023-06-07T05:35:34Z

tokens/utils.js

@@ -162,7 +162,37 @@ async function transformInPath(location, variablesMap, transformType = 'definiti
  }
 }

+
+function createIndexFile(buildDir, isTheme, themeVariant){


nit: tiny naming nit; perhaps something like createIndexCssFile just to be a tad bit more explicit in what type of file is being created by this function?

adamstankiewicz · 2023-06-07T05:44:20Z

tokens/utils.js

+    return;
+  }
+
+  const jsonFiles = files.filter(file => file !== 'index.css');


[clarification] Is this variable named jsonFiles because the resulting file names are the output from building the JSON design tokens? If so, this wasn't immediately obvious where for awhile I was thinking the files contains file names with .json file extensions but I don't think that's the case? If we're solely working with CSS files here, I wonder if there's a clearer name here, e.g. nonIndexCssFiles or outputCssFiles, etc.

adamstankiewicz · 2023-06-07T05:44:42Z

tokens/utils.js

+  }
+
+  const jsonFiles = files.filter(file => file !== 'index.css');
+  isTheme && jsonFiles.reverse();


It may be worth leaving a comment in the code explaining why the .reverse() is necessary.

I add a comment, it's not exactly necessary, it's just to maintain the order, I mean variables first when run build-scss.

A more explicit approach could be the following:

outputCssFiles.sort((a,b) => a.includes('variables') ? -1 : 0)

codecov · 2023-06-07T21:35:53Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.58%. Comparing base (73335b9) to head (95f1134).
Report is 878 commits behind head on alpha.

Additional details and impacted files

@@           Coverage Diff           @@
##            alpha    #2357   +/-   ##
=======================================
  Coverage   91.58%   91.58%           
=======================================
  Files         214      214           
  Lines        3519     3519           
  Branches      852      852           
=======================================
  Hits         3223     3223           
  Misses        288      288           
  Partials        8        8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

adamstankiewicz

Great work, lgtm! Appreciate the contribution 🙌 💯

openedx-webhooks · 2023-06-07T23:32:47Z

@dcoa 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

edx-semantic-release · 2023-06-07T23:50:48Z

🎉 This PR is included in version 21.0.0-alpha.37 🎉

The release is available on:

Your semantic-release bot 📦🚀

edx-semantic-release · 2023-09-01T12:55:26Z

🎉 This PR is included in version 22.0.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

openedx-semantic-release-bot · 2024-05-14T15:51:34Z

🎉 This PR is included in version 23.0.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

openedx-semantic-release-bot · 2024-12-12T02:37:47Z

🎉 This PR is included in version 23.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jun 5, 2023

feat: create themes flag to build tokens

88b1522

dcoa force-pushed the dcoa/paragon-css-variables branch from 6cb3004 to 88b1522 Compare June 6, 2023 00:01

adamstankiewicz reviewed Jun 7, 2023

View reviewed changes

fix: variables and functions names

95f1134

mphilbrick211 added the needs test run Author's first PR to this repository, awaiting test authorization from Axim label Jun 7, 2023

e0d removed the needs test run Author's first PR to this repository, awaiting test authorization from Axim label Jun 7, 2023

mphilbrick211 requested a review from adamstankiewicz June 7, 2023 23:16

adamstankiewicz approved these changes Jun 7, 2023

View reviewed changes

adamstankiewicz merged commit 13c53d0 into openedx:alpha Jun 7, 2023

edx-semantic-release added the released on @alpha label Jun 7, 2023

monteri pushed a commit to raccoongang/paragon that referenced this pull request Aug 17, 2023

feat: create themes flag to build tokens (openedx#2357)

efc1687

PKulkoRaccoonGang pushed a commit to raccoongang/paragon that referenced this pull request Aug 18, 2023

feat: create themes flag to build tokens (openedx#2357)

ad28b6a

PKulkoRaccoonGang pushed a commit that referenced this pull request Aug 1, 2024

feat: create themes flag to build tokens (#2357)

c10edc7

PKulkoRaccoonGang pushed a commit that referenced this pull request Aug 4, 2024

feat: create themes flag to build tokens (#2357)

ff89afa

PKulkoRaccoonGang pushed a commit that referenced this pull request Aug 4, 2024

feat: create themes flag to build tokens (#2357)

e0aa92e

PKulkoRaccoonGang pushed a commit that referenced this pull request Aug 5, 2024

feat: create themes flag to build tokens (#2357)

8d58733

openedx-semantic-release-bot added the released label Dec 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: create themes flag to build tokens #2357

feat: create themes flag to build tokens #2357

dcoa commented Jun 5, 2023 •

edited

Loading

openedx-webhooks commented Jun 5, 2023

adamstankiewicz left a comment

adamstankiewicz Jun 7, 2023

adamstankiewicz Jun 7, 2023

adamstankiewicz Jun 7, 2023

adamstankiewicz Jun 7, 2023

adamstankiewicz Jun 7, 2023

adamstankiewicz Jun 7, 2023

adamstankiewicz Jun 7, 2023

dcoa Jun 7, 2023

codecov bot commented Jun 7, 2023 •

edited

Loading

adamstankiewicz left a comment

openedx-webhooks commented Jun 7, 2023

edx-semantic-release commented Jun 7, 2023

edx-semantic-release commented Sep 1, 2023

openedx-semantic-release-bot commented May 14, 2024

openedx-semantic-release-bot commented Dec 12, 2024

	const THEME_VARIANTS = themes ? themes : ['light'];
	const THEME_VARIANTS = themes \|\| ['light'];

	.option('--themes <themes...>', 'A list of all the themes do you want to build, by default Paragn build light theme.')
	.option('--themes <themes...>', 'A list of theme variants to build. By default, Paragon currently only supports a light theme.')

feat: create themes flag to build tokens #2357

feat: create themes flag to build tokens #2357

Conversation

dcoa commented Jun 5, 2023 • edited Loading

Description

Merge Checklist

Post-merge Checklist

openedx-webhooks commented Jun 5, 2023

adamstankiewicz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 7, 2023 • edited Loading

Codecov Report

adamstankiewicz left a comment

Choose a reason for hiding this comment

openedx-webhooks commented Jun 7, 2023

edx-semantic-release commented Jun 7, 2023

edx-semantic-release commented Sep 1, 2023

openedx-semantic-release-bot commented May 14, 2024

openedx-semantic-release-bot commented Dec 12, 2024

dcoa commented Jun 5, 2023 •

edited

Loading

codecov bot commented Jun 7, 2023 •

edited

Loading