[Theming] Convert Component Usage (Part I) #467

michaeljaltamirano · 2020-10-28T19:16:11Z

What & Why

This PR starts moving over component styling usage from direct COLORS imports to pulling from the theme object.

It handles a fair number of the trivial cases:

Direct COLORS property access within component logic.
Direct COLORS property access within styled components.

Since it passed the Chromatic visual testing, I think it is safe to merge in, and so am tagging for review.

Next Steps

Deprecate direct COLORS property access within stand-alone css definitions.
- We need access to a theme object--we'll either in-line css SerializedStyles that we're currently nesting within styled components, or we'll set up functions to grant access to the theme object.
- This will likely be a much larger diff, and so I wanted to get an early PR in before tackling that work.
Figure out a plan for handling very loose Colors usage
- Example: our buttons accept any buttonColor prop. We need to tighten it to only be aliases present in a theme, or something else once we work through it.

…of distinction (for now)

michaeljaltamirano · 2020-10-28T19:21:32Z

yarn.lock

@@ -2110,6 +2110,17 @@
    react-syntax-highlighter "^12.2.1"
    regenerator-runtime "^0.13.3"

+"@storybook/addon-toolbars@^6.0.27":


I added this in another PR but accidentally removed it in the #454 bump. This adds it back.

michaeljaltamirano · 2020-10-28T19:24:06Z

src/shared-components/accordion/style.ts

  border: 1px solid ${theme.COLORS.border};

  ${ExpansionWrapper} {
-    ${isOpen && `border-top: 1px solid ${theme.COLORS.border}`};
+    ${isOpen ? `border-top: 1px solid ${theme.COLORS.border};` : ''}


Since I removed the serialized style, I inadvertently broke some sanitizing of the return object css was providing us. Now we explicitly add this semi-colon.

michaeljaltamirano · 2020-10-28T19:24:52Z

src/shared-components/accordion/test.tsx

+      <DecoratedAccordion {...testAccordionProps} onClick={spy} />,
+    );
+
+    const title = component.find('div[role="button"]');


The way we were accessing the title was quite brittle, and adding a higher order component blew up the test. I relied on mount in conjunction with find to make it more robust.

michaeljaltamirano · 2020-10-28T19:27:48Z

.size-snapshot.json

-    "gzipped": 5533,
+    "bundled": 35497,
+    "minified": 34532,
+    "gzipped": 5262,


Interestingly, removing the css SerializedStyles in styled components appears to bloat the size a bit. Removing some of those instances accounts for the change in this PR.

michaeljaltamirano · 2020-10-28T19:29:02Z

babel.config.js

@@ -6,4 +6,9 @@ module.exports = {
    ['@babel/plugin-proposal-class-properties', { loose: true }],
    ['@babel/plugin-transform-parameters', { loose: true }],
  ],
+  env: {
+    test: {
+      presets: ['@babel/preset-typescript'],


This was required to get tests/utils/decorateWithThemeProvider.tsx working. It's not really taking advantage of things right now but if we add more typed utilities it might.

michaeljaltamirano · 2020-10-28T19:30:37Z

src/constants/colors/secondary.ts

  primaryTint3: '#ff0000',
  secondary: '#ff8000',
-  tertiary: null,
+  tertiary: '',


The svg.fill type is string | undefined, so null was causing problems. Adding an empty string means we maintain some styles like border: 1px solid (which defaults to black), whereas undefined would add the style border: 1px solid undefined, which does not apply.

This is all a TODO, anyway, but this is just an FYI.

michaeljaltamirano · 2020-10-28T19:34:51Z

src/shared-components/avatar/style.ts

@@ -17,9 +15,11 @@ const determineSize = ({
  width: ${SIZES[avatarSize]}px;
 `;

-const AvatarImage = styled.img`
+const AvatarImage = styled.img<{
+  avatarSize: 'small' | 'medium' | 'large';


As an FYI as we convert things over, since I hadn't added types to this (which was never flagged since ${determineSize} pulled in the props implicitly), I saw a ThemeType-related error. Once I added this typing for avatarSize the compiler was able to interpret the theme typing correctly.

daigof

👍

* [Theme setup] Initial setup and Accordion example (#436) * initial setup * convert Acoordion,add theme suport to stories * complete accordion example * shouldShowForEnvironment refactor * commit snapshot size * v13.0.0-alpha.1 * Replace swap button with addon-toolbars to fix visual regression Co-authored-by: Michael Altamirano <michaeljaltamirano@gmail.com> * [Theming] Add built-in TypeScript support (#463) * initial setup * convert Acoordion,add theme suport to stories * complete accordion example * shouldShowForEnvironment refactor * commit snapshot size * v13.0.0-alpha.1 * Commit example with Rollup + TypeScript resolution * Commit hard-coded import * Revert "Commit hard-coded import" This reverts commit 9713df2. * PR Cleanup * Remove rollup plugin * Move type resolution files out of utils and into types folder * Commit master size-snapshot * Push last size-snapshot Co-authored-by: Diego Fortes <daigof@gmail.com> * [Theming] Convert Component Usage (Part I) (#467) * Convert for Accordion.Thumbnails * Convert easy cases: styled component and normal component usage * Add @types/enzyme-adapter-react-16 * Pass in blank string instead of undefined to maintain some semblance of distinction (for now) * Convert setupTests.js to ts * Get tests passing, check on chromatic * [Theming] Convert Component Usage (Part II, Buttons) (#475) * Bump @types/react from 16.9.53 to 16.9.55 (#473) * Bump @types/react-dom from 16.9.8 to 16.9.9 (#469) * Bump @types/enzyme from 3.10.7 to 3.10.8 (#470) * Bump lint-staged from 10.4.0 to 10.5.0 (#468) * Rename primary and secondary files to match default export name, export new COLORS_PROP_TYPES for aliased values * Export values as const for type inference, add secondary tints for lavender usage in buttons * Button update: add theming, constrict typing, remove unnecessary css imports (when in-lining in styled component) * Update size snapshot, new items due to new COLORS_PROP_TYPES usage * Decorate tests * Add back base button css composition, since typography depends on it * Tighten textColor typing, too * Override prettier, TODO: use prettier-eslint Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * [Theming] Convert Components (Part III) (#476) * Update global styles and storybook preview configuration * Update Callout, Chip, LoadingSpinner, and ProgressBar * Update DialogModal, Tabs, and Toggle * Update Dropdown * Update SelectorButton styles, modify eslint rule * Convert COntainer, ImmersiveModal, OptionButton * Last commits: Container and Field, and update .size-snapshot * Rename prop to get off DOM * [Theming] Convert Components (Part IV) (#477) * Bump @types/react from 16.9.53 to 16.9.55 (#473) * Bump @types/react-dom from 16.9.8 to 16.9.9 (#469) * Bump @types/enzyme from 3.10.7 to 3.10.8 (#470) * Bump lint-staged from 10.4.0 to 10.5.0 (#468) * Rename primary and secondary files to match default export name, export new COLORS_PROP_TYPES for aliased values * Export values as const for type inference, add secondary tints for lavender usage in buttons * Button update: add theming, constrict typing, remove unnecessary css imports (when in-lining in styled component) * Update size snapshot, new items due to new COLORS_PROP_TYPES usage * Decorate tests * Add back base button css composition, since typography depends on it * Tighten textColor typing, too * Override prettier, TODO: use prettier-eslint * Commit Typography WIP, save for last * Fix build, update .size-snapshot, replace all TYPOGRAPHY_STYLE usages, including story * Reconfigure stories/tsconfig, export ThemeColors instead for improved DX * Fix bizarre formatting * Fix formatting pt. 2 * One last eslint fix and prettier write cleanup * try running through eslint-plugin-prettier * Conform to docs order * Remove floating semicolons Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * [Theming] Update Colors Export (#480) * Bump @types/react from 16.9.53 to 16.9.55 (#473) * Bump @types/react-dom from 16.9.8 to 16.9.9 (#469) * Bump @types/enzyme from 3.10.7 to 3.10.8 (#470) * Bump lint-staged from 10.4.0 to 10.5.0 (#468) * Bump typescript from 4.0.3 to 4.0.5 (#471) * Bump chromatic from 5.2.0 to 5.3.0 (#478) * Bump @react-aria/focus from 3.2.2 to 3.2.3 (#479) * Mass commit: add TODO to existing COLORS export, fix lingering COLORS imports, update stories * Add react-is as devDependency to fix install warning * Fix visual regression in Available Constants story Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * v13.0.0-alpha.2 * [Theming] Fix tsc output (#498) * Re-arrange new files for theming to fix lib type file structure * Fix window.matchMedia type by fleshing it out * v13.0.0-alpha.3 * [Theming] Update Test Suite (#505) * [Theming] Update Declarations (#504) * Merge declaration changes from master * Rename code comment * Do not make Button onClick event potentially undefined * Keep Dropdown value type consistent * v13.0.0-alpha.4 * Push size changes, too * Narrow ReactNode to JSX.Element in ImmersiveModal title * Simplify generics usage * Increase carousel story delay to prevent load jank from failing regression test Co-authored-by: Diego Fortes <daigof@gmail.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>

* [Theme setup] Initial setup and Accordion example (#436) * initial setup * convert Acoordion,add theme suport to stories * complete accordion example * shouldShowForEnvironment refactor * commit snapshot size * v13.0.0-alpha.1 * Replace swap button with addon-toolbars to fix visual regression Co-authored-by: Michael Altamirano <michaeljaltamirano@gmail.com> * [Theming] Add built-in TypeScript support (#463) * initial setup * convert Acoordion,add theme suport to stories * complete accordion example * shouldShowForEnvironment refactor * commit snapshot size * v13.0.0-alpha.1 * Commit example with Rollup + TypeScript resolution * Commit hard-coded import * Revert "Commit hard-coded import" This reverts commit 9713df2. * PR Cleanup * Remove rollup plugin * Move type resolution files out of utils and into types folder * Commit master size-snapshot * Push last size-snapshot Co-authored-by: Diego Fortes <daigof@gmail.com> * [Theming] Convert Component Usage (Part I) (#467) * Convert for Accordion.Thumbnails * Convert easy cases: styled component and normal component usage * Add @types/enzyme-adapter-react-16 * Pass in blank string instead of undefined to maintain some semblance of distinction (for now) * Convert setupTests.js to ts * Get tests passing, check on chromatic * [Theming] Convert Component Usage (Part II, Buttons) (#475) * Bump @types/react from 16.9.53 to 16.9.55 (#473) * Bump @types/react-dom from 16.9.8 to 16.9.9 (#469) * Bump @types/enzyme from 3.10.7 to 3.10.8 (#470) * Bump lint-staged from 10.4.0 to 10.5.0 (#468) * Rename primary and secondary files to match default export name, export new COLORS_PROP_TYPES for aliased values * Export values as const for type inference, add secondary tints for lavender usage in buttons * Button update: add theming, constrict typing, remove unnecessary css imports (when in-lining in styled component) * Update size snapshot, new items due to new COLORS_PROP_TYPES usage * Decorate tests * Add back base button css composition, since typography depends on it * Tighten textColor typing, too * Override prettier, TODO: use prettier-eslint Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * [Theming] Convert Components (Part III) (#476) * Update global styles and storybook preview configuration * Update Callout, Chip, LoadingSpinner, and ProgressBar * Update DialogModal, Tabs, and Toggle * Update Dropdown * Update SelectorButton styles, modify eslint rule * Convert COntainer, ImmersiveModal, OptionButton * Last commits: Container and Field, and update .size-snapshot * Rename prop to get off DOM * [Theming] Convert Components (Part IV) (#477) * Bump @types/react from 16.9.53 to 16.9.55 (#473) * Bump @types/react-dom from 16.9.8 to 16.9.9 (#469) * Bump @types/enzyme from 3.10.7 to 3.10.8 (#470) * Bump lint-staged from 10.4.0 to 10.5.0 (#468) * Rename primary and secondary files to match default export name, export new COLORS_PROP_TYPES for aliased values * Export values as const for type inference, add secondary tints for lavender usage in buttons * Button update: add theming, constrict typing, remove unnecessary css imports (when in-lining in styled component) * Update size snapshot, new items due to new COLORS_PROP_TYPES usage * Decorate tests * Add back base button css composition, since typography depends on it * Tighten textColor typing, too * Override prettier, TODO: use prettier-eslint * Commit Typography WIP, save for last * Fix build, update .size-snapshot, replace all TYPOGRAPHY_STYLE usages, including story * Reconfigure stories/tsconfig, export ThemeColors instead for improved DX * Fix bizarre formatting * Fix formatting pt. 2 * One last eslint fix and prettier write cleanup * try running through eslint-plugin-prettier * Conform to docs order * Remove floating semicolons Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * [Theming] Update Colors Export (#480) * Bump @types/react from 16.9.53 to 16.9.55 (#473) * Bump @types/react-dom from 16.9.8 to 16.9.9 (#469) * Bump @types/enzyme from 3.10.7 to 3.10.8 (#470) * Bump lint-staged from 10.4.0 to 10.5.0 (#468) * Bump typescript from 4.0.3 to 4.0.5 (#471) * Bump chromatic from 5.2.0 to 5.3.0 (#478) * Bump @react-aria/focus from 3.2.2 to 3.2.3 (#479) * Mass commit: add TODO to existing COLORS export, fix lingering COLORS imports, update stories * Add react-is as devDependency to fix install warning * Fix visual regression in Available Constants story Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * v13.0.0-alpha.2 * [Theming] Fix tsc output (#498) * Re-arrange new files for theming to fix lib type file structure * Fix window.matchMedia type by fleshing it out * v13.0.0-alpha.3 * [Theming] Update Test Suite (#505) * [Theming] Update Declarations (#504) * Merge declaration changes from master * Rename code comment * Do not make Button onClick event potentially undefined * Keep Dropdown value type consistent * v13.0.0-alpha.4 * Push size changes, too * Narrow ReactNode to JSX.Element in ImmersiveModal title * Simplify generics usage * refactor fonts to theme * add test * refactor out test util function * reafctor util tester * [TypeScript] Replace ThemeType Absolute Imports with Relative Imports (#510) * Move ThemeType (and associated imports) to relative imports instead of absolute * Update .size-snapshot.json Co-authored-by: Michael Altamirano <michaeljaltamirano@gmail.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>

* [Themes] - Add fonts to themes (#509) * [Theme setup] Initial setup and Accordion example (#436) * initial setup * convert Acoordion,add theme suport to stories * complete accordion example * shouldShowForEnvironment refactor * commit snapshot size * v13.0.0-alpha.1 * Replace swap button with addon-toolbars to fix visual regression Co-authored-by: Michael Altamirano <michaeljaltamirano@gmail.com> * [Theming] Add built-in TypeScript support (#463) * initial setup * convert Acoordion,add theme suport to stories * complete accordion example * shouldShowForEnvironment refactor * commit snapshot size * v13.0.0-alpha.1 * Commit example with Rollup + TypeScript resolution * Commit hard-coded import * Revert "Commit hard-coded import" This reverts commit 9713df2. * PR Cleanup * Remove rollup plugin * Move type resolution files out of utils and into types folder * Commit master size-snapshot * Push last size-snapshot Co-authored-by: Diego Fortes <daigof@gmail.com> * [Theming] Convert Component Usage (Part I) (#467) * Convert for Accordion.Thumbnails * Convert easy cases: styled component and normal component usage * Add @types/enzyme-adapter-react-16 * Pass in blank string instead of undefined to maintain some semblance of distinction (for now) * Convert setupTests.js to ts * Get tests passing, check on chromatic * [Theming] Convert Component Usage (Part II, Buttons) (#475) * Bump @types/react from 16.9.53 to 16.9.55 (#473) * Bump @types/react-dom from 16.9.8 to 16.9.9 (#469) * Bump @types/enzyme from 3.10.7 to 3.10.8 (#470) * Bump lint-staged from 10.4.0 to 10.5.0 (#468) * Rename primary and secondary files to match default export name, export new COLORS_PROP_TYPES for aliased values * Export values as const for type inference, add secondary tints for lavender usage in buttons * Button update: add theming, constrict typing, remove unnecessary css imports (when in-lining in styled component) * Update size snapshot, new items due to new COLORS_PROP_TYPES usage * Decorate tests * Add back base button css composition, since typography depends on it * Tighten textColor typing, too * Override prettier, TODO: use prettier-eslint Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * [Theming] Convert Components (Part III) (#476) * Update global styles and storybook preview configuration * Update Callout, Chip, LoadingSpinner, and ProgressBar * Update DialogModal, Tabs, and Toggle * Update Dropdown * Update SelectorButton styles, modify eslint rule * Convert COntainer, ImmersiveModal, OptionButton * Last commits: Container and Field, and update .size-snapshot * Rename prop to get off DOM * [Theming] Convert Components (Part IV) (#477) * Bump @types/react from 16.9.53 to 16.9.55 (#473) * Bump @types/react-dom from 16.9.8 to 16.9.9 (#469) * Bump @types/enzyme from 3.10.7 to 3.10.8 (#470) * Bump lint-staged from 10.4.0 to 10.5.0 (#468) * Rename primary and secondary files to match default export name, export new COLORS_PROP_TYPES for aliased values * Export values as const for type inference, add secondary tints for lavender usage in buttons * Button update: add theming, constrict typing, remove unnecessary css imports (when in-lining in styled component) * Update size snapshot, new items due to new COLORS_PROP_TYPES usage * Decorate tests * Add back base button css composition, since typography depends on it * Tighten textColor typing, too * Override prettier, TODO: use prettier-eslint * Commit Typography WIP, save for last * Fix build, update .size-snapshot, replace all TYPOGRAPHY_STYLE usages, including story * Reconfigure stories/tsconfig, export ThemeColors instead for improved DX * Fix bizarre formatting * Fix formatting pt. 2 * One last eslint fix and prettier write cleanup * try running through eslint-plugin-prettier * Conform to docs order * Remove floating semicolons Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * [Theming] Update Colors Export (#480) * Bump @types/react from 16.9.53 to 16.9.55 (#473) * Bump @types/react-dom from 16.9.8 to 16.9.9 (#469) * Bump @types/enzyme from 3.10.7 to 3.10.8 (#470) * Bump lint-staged from 10.4.0 to 10.5.0 (#468) * Bump typescript from 4.0.3 to 4.0.5 (#471) * Bump chromatic from 5.2.0 to 5.3.0 (#478) * Bump @react-aria/focus from 3.2.2 to 3.2.3 (#479) * Mass commit: add TODO to existing COLORS export, fix lingering COLORS imports, update stories * Add react-is as devDependency to fix install warning * Fix visual regression in Available Constants story Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * v13.0.0-alpha.2 * [Theming] Fix tsc output (#498) * Re-arrange new files for theming to fix lib type file structure * Fix window.matchMedia type by fleshing it out * v13.0.0-alpha.3 * [Theming] Update Test Suite (#505) * [Theming] Update Declarations (#504) * Merge declaration changes from master * Rename code comment * Do not make Button onClick event potentially undefined * Keep Dropdown value type consistent * v13.0.0-alpha.4 * Push size changes, too * Narrow ReactNode to JSX.Element in ImmersiveModal title * Simplify generics usage * refactor fonts to theme * add test * refactor out test util function * reafctor util tester * [TypeScript] Replace ThemeType Absolute Imports with Relative Imports (#510) * Move ThemeType (and associated imports) to relative imports instead of absolute * Update .size-snapshot.json Co-authored-by: Michael Altamirano <michaeljaltamirano@gmail.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * [Colors] Add 'transparent' color (#512) * Add transparent as valid COLORS alias/value Co-authored-by: Michael Altamirano <michaeljaltamirano@gmail.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>

michaeljaltamirano added 6 commits October 28, 2020 11:03

Convert for Accordion.Thumbnails

b142b6f

Convert easy cases: styled component and normal component usage

30f3217

Add @types/enzyme-adapter-react-16

fd5381a

Pass in blank string instead of undefined to maintain some semblance …

4d9ffde

…of distinction (for now)

Convert setupTests.js to ts

a5e141e

Get tests passing, check on chromatic

2f4194d

snags88 deployed to curology-radiance-pr-467 October 28, 2020 19:16 View deployment

michaeljaltamirano changed the base branch from master to epic/add-theming-functionality October 28, 2020 19:17

michaeljaltamirano commented Oct 28, 2020

View reviewed changes

michaeljaltamirano requested a review from daigof October 28, 2020 19:25

michaeljaltamirano commented Oct 28, 2020

View reviewed changes

daigof approved these changes Oct 28, 2020

View reviewed changes

michaeljaltamirano merged commit 3731fa7 into epic/add-theming-functionality Oct 28, 2020

michaeljaltamirano deleted the theming/convert-components branch October 28, 2020 20:24

daigof mentioned this pull request Nov 9, 2020

[Theming] - Add FONTS support and expand COLORS object #511

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Theming] Convert Component Usage (Part I) #467

[Theming] Convert Component Usage (Part I) #467

michaeljaltamirano commented Oct 28, 2020 •

edited

Loading

michaeljaltamirano Oct 28, 2020

michaeljaltamirano Oct 28, 2020

michaeljaltamirano Oct 28, 2020

michaeljaltamirano Oct 28, 2020

michaeljaltamirano Oct 28, 2020

michaeljaltamirano Oct 28, 2020

michaeljaltamirano Oct 28, 2020

daigof left a comment

[Theming] Convert Component Usage (Part I) #467

[Theming] Convert Component Usage (Part I) #467

Conversation

michaeljaltamirano commented Oct 28, 2020 • edited Loading

What & Why

Next Steps

michaeljaltamirano Oct 28, 2020

Choose a reason for hiding this comment

michaeljaltamirano Oct 28, 2020

Choose a reason for hiding this comment

michaeljaltamirano Oct 28, 2020

Choose a reason for hiding this comment

michaeljaltamirano Oct 28, 2020

Choose a reason for hiding this comment

michaeljaltamirano Oct 28, 2020

Choose a reason for hiding this comment

michaeljaltamirano Oct 28, 2020

Choose a reason for hiding this comment

michaeljaltamirano Oct 28, 2020

Choose a reason for hiding this comment

daigof left a comment

Choose a reason for hiding this comment

michaeljaltamirano commented Oct 28, 2020 •

edited

Loading