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(themes): enable dynamic values for interactive color tokens #5269

Merged

Conversation

vpicone
Copy link
Contributor

@vpicone vpicone commented Feb 5, 2020

Closes #4049

Today, interactive UI colors (e.g. hover-danger) are hard-coded in to the themes. This means when the color pallet was updated, all of the interactive tokens became outdated. They're still currently the static values generated from the v1 color pallet.

Changelog

New

/**
 * Example: token = hsl(10, 10, 10);
 * adjustLightness(token, 5) === hsl(10, 10, 15);
 * adjustLightness(token, -5) === hsl(10, 10, 5);
 * @param {string} token
 * @param {integer} shift The number of percentage points (positive or negative) by which to shift the lightness of a token.
 * @returns {string}
 */
export function adjustLightness(token, shift)

Changed

  • Use adjustLightness for hover-danger tokens as proof of concept.

Testing
Compare generated hex values to the hex values in the v2 sketch kit. You can select the square and view the hex code in sketch's color selector.

@vpicone vpicone requested a review from a team as a code owner February 5, 2020 03:09
@ghost ghost requested review from joshblack and tw15egan February 5, 2020 03:09
@netlify
Copy link

netlify bot commented Feb 5, 2020

Deploy preview for carbon-elements ready!

Built with commit 233a67e

https://deploy-preview-5269--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Feb 5, 2020

Deploy preview for carbon-components-react ready!

Built with commit 233a67e

https://deploy-preview-5269--carbon-components-react.netlify.com

@vpicone vpicone changed the title feat: enable dynamic values for interactive tokens feat: enable dynamic values for interactive color tokens Feb 5, 2020
@vpicone
Copy link
Contributor Author

vpicone commented Feb 5, 2020

Happy birthday @sadekbazaraa

@vpicone
Copy link
Contributor Author

vpicone commented Feb 5, 2020

I can use the function for the rest of the interactive tokens if we like this approach.

@vpicone vpicone requested review from emyarod and removed request for joshblack February 6, 2020 21:37
@vpicone vpicone requested review from joshblack and emyarod February 10, 2020 16:55
Copy link
Member

@emyarod emyarod left a comment

Choose a reason for hiding this comment

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

thanks for adding the tests!

@emyarod emyarod self-requested a review February 10, 2020 17:17
Copy link
Member

@emyarod emyarod left a comment

Choose a reason for hiding this comment

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

it looks like these tests are failing locally

FAIL  packages/themes/__tests__/themes-test.js (7.658s)
  ● themes.scss › tokens › hoverDanger should match the default theme

    expect(received).toEqual(expected) // deep equality

    Expected: "#b81921"
    Received: "#ba1b23"

      55 |         // param-case to camelCase and that values are correctly mapped over for
      56 |         // strings and numbers
    > 57 |         expect(formatObjectKeys(convert(calls[1][0]))).toEqual(
         |                                                        ^
      58 |           defaultTheme[token]
      59 |         );
      60 |       }

      at _callee$ (packages/themes/__tests__/themes-test.js:57:56)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:45:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:271:22)
      at Generator.prototype.(anonymous function) [as next] (node_modules/regenerator-runtime/runtime.js:97:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at _next (node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)

  ● themes.scss › carbon--theme › should export tokens that match the default theme

    expect(received).toEqual(expected) // deep equality

    Expected: "#ba1b23"
    Received: "#b81921"

      71 |
      72 |       Object.keys(defaultTheme).forEach(token => {
    > 73 |         expect(defaultTheme[token]).toEqual(
         |                                     ^
      74 |           formatObjectKeys(theme[formatTokenName(token)])
      75 |         );
      76 |       });

      at forEach (packages/themes/__tests__/themes-test.js:73:37)
          at Array.forEach (<anonymous>)
      at _callee2$ (packages/themes/__tests__/themes-test.js:72:33)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:45:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:271:22)
      at Generator.prototype.(anonymous function) [as next] (node_modules/regenerator-runtime/runtime.js:97:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at _next (node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)

@emyarod emyarod mentioned this pull request Feb 10, 2020
2 tasks

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@vpicone
Copy link
Contributor Author

vpicone commented Feb 10, 2020

@emyarod you have to run build inside the theme package and generate new scss.

@emyarod
Copy link
Member

emyarod commented Feb 10, 2020

thanks, I guess my yarn install, build, and test commands were out of order

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@joshblack joshblack changed the title feat: enable dynamic values for interactive color tokens feat(themes): enable dynamic values for interactive color tokens Feb 17, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@asudoh asudoh merged commit 437da98 into carbon-design-system:master Feb 18, 2020
@joshblack joshblack mentioned this pull request Feb 25, 2020
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Tokens] Implement lighten/darken method for color change across states. Deprecate Hover tokens.
4 participants