From d24e9ed84b70b53d1741e35908e854c69bae8f62 Mon Sep 17 00:00:00 2001 From: manajdov Date: Tue, 4 Dec 2018 13:43:51 +0100 Subject: [PATCH 1/5] wip --- src/components/Provider/Provider.tsx | 34 ++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/src/components/Provider/Provider.tsx b/src/components/Provider/Provider.tsx index 50ee52d236..34f8b944b4 100644 --- a/src/components/Provider/Provider.tsx +++ b/src/components/Provider/Provider.tsx @@ -11,6 +11,8 @@ import { StaticStyle, StaticStyleFunction, FontFace, + ThemeComponentVariablesPrepared, + ComponentVariablesPrepared, } from '../../themes/types' import ProviderConsumer from './ProviderConsumer' import { mergeSiteVariables } from '../../lib/mergeThemes' @@ -20,10 +22,20 @@ export interface ProviderProps { children: React.ReactNode } +export interface ProviderState { + staticStyles?: StaticStyle[] + siteVariables?: { [key in keyof ThemeComponentVariablesPrepared]: ComponentVariablesPrepared } +} + /** * The Provider passes the CSS in JS renderer and theme to your components. */ -class Provider extends React.Component { +class Provider extends React.Component { + constructor(props: ProviderProps) { + super(props) + // this.state = {} + } + static propTypes = { theme: PropTypes.shape({ siteVariables: PropTypes.object, @@ -54,15 +66,22 @@ class Provider extends React.Component { static Consumer = ProviderConsumer - renderStaticStyles = () => { + renderStaticStyles = (outgoingTheme: ThemePrepared) => { // RTL WARNING // This function sets static styles which are global and renderer agnostic // With current implementation, static styles cannot differ between LTR and RTL // @see http://fela.js.org/docs/advanced/StaticStyle.html for details - const { siteVariables, staticStyles } = this.props.theme + const { siteVariables } = outgoingTheme + const { staticStyles } = this.props.theme if (!staticStyles) return + // + // const {staticStyles: stateStaticStyles, siteVariables: stateStaticVariables} = this.state + // + // if (stateStaticStyles === staticStyles && stateStaticVariables === siteVariables) return + + console.log('Rendering static styles...') const renderObject = (object: StaticStyleObject) => { _.forEach(object, (style, selector) => { @@ -84,6 +103,11 @@ class Provider extends React.Component { ) } }) + // + // this.setState({ + // staticStyles, + // siteVariables, + // }) } renderFontFaces = () => { @@ -109,7 +133,7 @@ class Provider extends React.Component { } componentDidMount() { - this.renderStaticStyles() + // this.renderStaticStyles() this.renderFontFaces() } @@ -122,7 +146,7 @@ class Provider extends React.Component { { const outgoingTheme: ThemePrepared = mergeThemes(incomingTheme, theme) - + this.renderStaticStyles(outgoingTheme) return ( {children} From 5cb85fdbc4fb7908d4ca06142bfc917c4248ebab Mon Sep 17 00:00:00 2001 From: manajdov Date: Wed, 5 Dec 2018 14:49:26 +0100 Subject: [PATCH 2/5] -added logic for rendering the staticStyles if they are provided, just once in one Provider instance. --- src/components/Provider/Provider.tsx | 39 +++++++++++----------------- 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/src/components/Provider/Provider.tsx b/src/components/Provider/Provider.tsx index 34f8b944b4..1025b0d4fd 100644 --- a/src/components/Provider/Provider.tsx +++ b/src/components/Provider/Provider.tsx @@ -11,8 +11,6 @@ import { StaticStyle, StaticStyleFunction, FontFace, - ThemeComponentVariablesPrepared, - ComponentVariablesPrepared, } from '../../themes/types' import ProviderConsumer from './ProviderConsumer' import { mergeSiteVariables } from '../../lib/mergeThemes' @@ -22,18 +20,15 @@ export interface ProviderProps { children: React.ReactNode } -export interface ProviderState { - staticStyles?: StaticStyle[] - siteVariables?: { [key in keyof ThemeComponentVariablesPrepared]: ComponentVariablesPrepared } -} - /** * The Provider passes the CSS in JS renderer and theme to your components. */ -class Provider extends React.Component { +class Provider extends React.Component { + staticStylesRendered: boolean + constructor(props: ProviderProps) { super(props) - // this.state = {} + this.staticStylesRendered = false } static propTypes = { @@ -66,22 +61,16 @@ class Provider extends React.Component { static Consumer = ProviderConsumer - renderStaticStyles = (outgoingTheme: ThemePrepared) => { + renderStaticStyles = (mergedTheme: ThemePrepared) => { // RTL WARNING // This function sets static styles which are global and renderer agnostic // With current implementation, static styles cannot differ between LTR and RTL // @see http://fela.js.org/docs/advanced/StaticStyle.html for details - const { siteVariables } = outgoingTheme + const { siteVariables } = mergedTheme const { staticStyles } = this.props.theme if (!staticStyles) return - // - // const {staticStyles: stateStaticStyles, siteVariables: stateStaticVariables} = this.state - // - // if (stateStaticStyles === staticStyles && stateStaticVariables === siteVariables) return - - console.log('Rendering static styles...') const renderObject = (object: StaticStyleObject) => { _.forEach(object, (style, selector) => { @@ -103,11 +92,6 @@ class Provider extends React.Component { ) } }) - // - // this.setState({ - // staticStyles, - // siteVariables, - // }) } renderFontFaces = () => { @@ -133,7 +117,6 @@ class Provider extends React.Component { } componentDidMount() { - // this.renderStaticStyles() this.renderFontFaces() } @@ -146,7 +129,7 @@ class Provider extends React.Component { { const outgoingTheme: ThemePrepared = mergeThemes(incomingTheme, theme) - this.renderStaticStyles(outgoingTheme) + this.renderStaticStylesOnce(outgoingTheme) return ( {children} @@ -156,6 +139,14 @@ class Provider extends React.Component { /> ) } + + renderStaticStylesOnce = (mergedTheme: ThemePrepared) => { + const { staticStyles } = this.props.theme + if (!this.staticStylesRendered && staticStyles) { + this.renderStaticStyles(mergedTheme) + this.staticStylesRendered = true + } + } } export default Provider From a52782bce242966f084f2464ce461c0b8259a5a4 Mon Sep 17 00:00:00 2001 From: manajdov Date: Wed, 5 Dec 2018 15:46:59 +0100 Subject: [PATCH 3/5] -updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a85218ae75..382d07711a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Fix truncate styles in Teams team for the `Button`'s `content` prop used as element @mnajdova ([#551](https://github.com/stardust-ui/react/pull/551)) - Fix HTML preview in the editor @layershifter ([#555](https://github.com/stardust-ui/react/pull/555)) - Fix icon overlapping for `iconOnly` prop in `Menu` component with @Bugaa92 ([#486](https://github.com/stardust-ui/react/pull/486)) +- Fix `Provider` is not executing staticStyles with the merged siteVariables @mnajdova ([#559](https://github.com/stardust-ui/react/pull/559)) ### Features - Add `render` callback as an option for shorthand value @kuzhelov ([#519](https://github.com/stardust-ui/react/pull/519)) From b60ebff8f6f0f67bd9e498bd9fdb8b83fcdc3649 Mon Sep 17 00:00:00 2001 From: manajdov Date: Wed, 5 Dec 2018 16:08:19 +0100 Subject: [PATCH 4/5] -added for ensuring that staticStyles are executed just once in the Provider -added test for ensuring that staticStyles are invoked with the merged siteVariables --- src/components/Provider/Provider.tsx | 7 +-- .../components/Provider/Provider-test.tsx | 44 +++++++++++++++++++ 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/components/Provider/Provider.tsx b/src/components/Provider/Provider.tsx index 1025b0d4fd..8d92333e3b 100644 --- a/src/components/Provider/Provider.tsx +++ b/src/components/Provider/Provider.tsx @@ -24,12 +24,7 @@ export interface ProviderProps { * The Provider passes the CSS in JS renderer and theme to your components. */ class Provider extends React.Component { - staticStylesRendered: boolean - - constructor(props: ProviderProps) { - super(props) - this.staticStylesRendered = false - } + staticStylesRendered: boolean = false static propTypes = { theme: PropTypes.shape({ diff --git a/test/specs/components/Provider/Provider-test.tsx b/test/specs/components/Provider/Provider-test.tsx index 4c3b185e0a..744972873f 100644 --- a/test/specs/components/Provider/Provider-test.tsx +++ b/test/specs/components/Provider/Provider-test.tsx @@ -1,5 +1,7 @@ +import * as React from 'react' import Provider from 'src/components/Provider/Provider' import ProviderConsumer from 'src/components/Provider/ProviderConsumer' +import { mount } from 'enzyme' describe('Provider', () => { test('is exported', () => { @@ -9,4 +11,46 @@ describe('Provider', () => { test('has a ProviderConsumer subcomponent', () => { expect(require('src/index.ts').Provider.Consumer).toEqual(ProviderConsumer) }) + + describe('staticStyles', () => { + test('are executed with the merged siteVariables', () => { + const staticStyle = jest.fn() + + mount( + + + + + , + ) + + expect(staticStyle).toHaveBeenCalledWith( + expect.objectContaining({ + background: 'red', + brand: 'yellow', + gray: '#868686', + }), + ) + }) + + test('are executed only once', () => { + const firstStaticStyle = jest.fn() + const secondStaticStyle = jest.fn() + + const providerInstance = mount( + + + , + ) + providerInstance.setProps({ theme: { staticStyles: [secondStaticStyle] } }) + + expect(firstStaticStyle).toHaveBeenCalledTimes(1) + expect(secondStaticStyle).not.toHaveBeenCalled() + }) + }) }) From 106af27c0ecbae13421f238af7a5c96825f1caba Mon Sep 17 00:00:00 2001 From: kuzhelov Date: Wed, 5 Dec 2018 22:53:46 +0100 Subject: [PATCH 5/5] adjust changelog entry --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bee5d29b43..19d913f989 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Fixes +- Fix `Provider` is not executing staticStyles with the merged siteVariables @mnajdova ([#559](https://github.com/stardust-ui/react/pull/559)) + ### Features - `Ref` components uses `forwardRef` API by default @layershifter ([#491](https://github.com/stardust-ui/react/pull/491)) @@ -48,7 +51,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Fix truncate styles in Teams team for the `Button`'s `content` prop used as element @mnajdova ([#551](https://github.com/stardust-ui/react/pull/551)) - Fix HTML preview in the editor @layershifter ([#555](https://github.com/stardust-ui/react/pull/555)) - Fix icon overlapping for `iconOnly` prop in `Menu` component with @Bugaa92 ([#486](https://github.com/stardust-ui/react/pull/486)) -- Fix `Provider` is not executing staticStyles with the merged siteVariables @mnajdova ([#559](https://github.com/stardust-ui/react/pull/559)) ### Features - Add `render` callback as an option for shorthand value @kuzhelov ([#519](https://github.com/stardust-ui/react/pull/519))