fix(factories): styles defined as func are not working (#470)

* -fix for supporting styles as functions in the shorthands * -updating factories tests * -introducing mergeStyles function and updating tests * -updated changelog
microsoft · Nov 14, 2018 · 61ee332 · 61ee332
1 parent 55c6f6a
commit 61ee332
Show file tree

Hide file tree

Showing 5 changed files with 138 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -27,6 +27,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Remove `Sizes` and `Weights` enums, use typed string in `Text` instead @jurokapsiar ([#446](https://github.com/stardust-ui/react/pull/446))
 - Fix React's version in `peerDependencies` @layershifter ([#452](https://github.com/stardust-ui/react/pull/452))
 - Added Dark and Contrast theme variables for `Header` @bcalvery ([#427](https://github.com/stardust-ui/react/pull/427))
+- Fix styles as functions in shorthands are not applied @mnajdova ([#470](https://github.com/stardust-ui/react/pull/470))
 
 ### Features
 - Make `Grid` keyboard navigable by implementing `gridBehavior` @sophieH29 ([#398](https://github.com/stardust-ui/react/pull/398))

diff --git a/src/lib/factories.tsx b/src/lib/factories.tsx
@@ -8,6 +8,7 @@ import {
   ShorthandValue,
   Props,
 } from '../../types/utils'
+import { mergeStyles } from './mergeThemes'
 
 interface CreateShorthandOptions {
   /** Override the default render implementation. */
@@ -106,7 +107,7 @@ export function createShorthand(
 
   // Merge styles
   if (defaultProps.styles || overrideProps.styles || usersProps.styles) {
-    props.styles = _.merge(defaultProps.styles, usersProps.styles, overrideProps.styles)
+    props.styles = mergeStyles(defaultProps.styles, usersProps.styles, overrideProps.styles)
   }
 
   // ----------------------------------------

diff --git a/src/lib/mergeThemes.ts b/src/lib/mergeThemes.ts
@@ -15,6 +15,7 @@ import {
   ThemePrepared,
   StaticStyle,
   ThemeIcons,
+  ComponentSlotStyle,
 } from '../themes/types'
 import callable from './callable'
 import { felaRenderer, felaRtlRenderer } from './felaRenderer'
@@ -176,6 +177,14 @@ export const mergeIcons = (target: ThemeIcons, ...sources: ThemeIcons[]): ThemeI
   return Object.assign(target, ...sources)
 }
 
+export const mergeStyles = (...sources: ComponentSlotStyle[]) => {
+  return (...args) => {
+    return sources.reduce((acc, next) => {
+      return _.merge(acc, callable(next)(...args))
+    }, {})
+  }
+}
+
 const mergeThemes = (...themes: ThemeInput[]): ThemePrepared => {
   const emptyTheme = {
     siteVariables: {},

diff --git a/test/specs/lib/factories-test.tsx b/test/specs/lib/factories-test.tsx
@@ -3,6 +3,7 @@ import { shallow } from 'enzyme'
 import * as React from 'react'
 import { createShorthand, createShorthandFactory } from 'src/lib'
 import { consoleUtil } from 'test/utils'
+import callable from '../../../src/lib/callable'
 
 // ----------------------------------------
 // Utils
@@ -229,7 +230,7 @@ describe('factories', () => {
           Component: 'p',
           defaultProps,
           render(Component, props) {
-            expect(props.styles).toMatchObject({
+            expect(callable(props.styles)()).toMatchObject({
               color: 'black',
               ':hover': { color: 'blue' },
             })
@@ -264,7 +265,72 @@ describe('factories', () => {
           Component: 'p',
           overrideProps,
           render(Component, props) {
-            expect(props.styles).toMatchObject({
+            expect(callable(props.styles)()).toMatchObject({
+              position: 'keep',
+              color: 'black',
+              ':hover': {
+                position: 'keep',
+                color: 'blue',
+              },
+            })
+          },
+        })
+      })
+
+      test('deep merges styles prop as function onto defaultProps styles', () => {
+        expect.assertions(1)
+
+        const defaultProps = {
+          styles: () => ({
+            color: 'override me',
+            ':hover': { color: 'blue' },
+          }),
+        }
+        const props = {
+          styles: { color: 'black' },
+        }
+
+        getShorthand({
+          value: props,
+          Component: 'p',
+          defaultProps,
+          render(Component, props) {
+            expect(callable(props.styles)()).toMatchObject({
+              color: 'black',
+              ':hover': { color: 'blue' },
+            })
+          },
+        })
+      })
+
+      test('deep merges overrideProps styles as function onto styles prop', () => {
+        expect.assertions(1)
+
+        const overrideProps = {
+          styles: () => ({
+            color: 'black',
+            ':hover': {
+              color: 'blue',
+            },
+          }),
+        }
+        const props = {
+          styles: {
+            position: 'keep',
+            color: 'override',
+            ':hover': {
+              position: 'keep',
+              color: 'override',
+            },
+          },
+        }
+
+        getShorthand({
+          value: props,
+          Component: 'p',
+          overrideProps,
+          render(Component, props) {
+            expect(callable(props.styles)()).toMatchObject({
               position: 'keep',
               color: 'black',
               ':hover': {

diff --git a/test/specs/lib/mergeThemes/mergeThemes-test.ts b/test/specs/lib/mergeThemes/mergeThemes-test.ts
@@ -1,5 +1,6 @@
-import mergeThemes from '../../../../src/lib/mergeThemes'
+import mergeThemes, { mergeStyles } from '../../../../src/lib/mergeThemes'
 import { felaRenderer, felaRtlRenderer } from '../../../../src/lib'
+import { ICSSInJSStyle } from '../../../../src/themes/types'
 
 describe('mergeThemes', () => {
   test(`always returns an object`, () => {
@@ -347,4 +348,60 @@ describe('mergeThemes', () => {
       expect(mergeThemes({ rtl: undefined })).toHaveProperty('renderer', felaRenderer)
     })
   })
+
+  describe('styles', () => {
+    test('merges styles object and function', () => {
+      const stylesAsObject: ICSSInJSStyle = {
+        margin: '0px',
+        color: 'override',
+        ':hover': {
+          margin: '0px',
+          color: 'override',
+        },
+      }
+
+      const stylesAsFunction = () => ({
+        color: 'black',
+        ':hover': {
+          color: 'blue',
+        },
+      })
+
+      expect(mergeStyles(stylesAsObject, stylesAsFunction)()).toMatchObject({
+        margin: '0px',
+        color: 'black',
+        ':hover': {
+          margin: '0px',
+          color: 'blue',
+        },
+      })
+    })
+
+    test('merges styles function and object', () => {
+      const stylesAsFunction = () => ({
+        margin: '0px',
+        color: 'override',
+        ':hover': {
+          margin: '0px',
+          color: 'override',
+        },
+      })
+
+      const stylesAsObject = {
+        color: 'black',
+        ':hover': {
+          color: 'blue',
+        },
+      }
+
+      expect(mergeStyles(stylesAsFunction, stylesAsObject)()).toMatchObject({
+        margin: '0px',
+        color: 'black',
+        ':hover': {
+          margin: '0px',
+          color: 'blue',
+        },
+      })
+    })
+  })
 })