Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

fix(Provider): staticStyles should be executed with the merged siteVariables #559

Merged
merged 7 commits into from
Dec 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
20 changes: 15 additions & 5 deletions src/components/Provider/Provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ export interface ProviderProps {
/**
* The Provider passes the CSS in JS renderer and theme to your components.
*/
class Provider extends React.Component<ProviderProps, any> {
class Provider extends React.Component<ProviderProps> {
staticStylesRendered: boolean = false

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
staticStylesRendered: boolean = false

You can set a value without constructor, or I am missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I will remove the constructor. I was trying different things, it is just leftover.

static propTypes = {
theme: PropTypes.shape({
siteVariables: PropTypes.object,
Expand Down Expand Up @@ -54,13 +56,14 @@ class Provider extends React.Component<ProviderProps, any> {

static Consumer = ProviderConsumer

renderStaticStyles = () => {
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, staticStyles } = this.props.theme
const { siteVariables } = mergedTheme
const { staticStyles } = this.props.theme

if (!staticStyles) return

Expand Down Expand Up @@ -109,7 +112,6 @@ class Provider extends React.Component<ProviderProps, any> {
}

componentDidMount() {
this.renderStaticStyles()
this.renderFontFaces()
}

Expand All @@ -122,7 +124,7 @@ class Provider extends React.Component<ProviderProps, any> {
<ProviderConsumer
render={(incomingTheme: ThemePrepared) => {
const outgoingTheme: ThemePrepared = mergeThemes(incomingTheme, theme)

this.renderStaticStylesOnce(outgoingTheme)
return (
<RendererProvider renderer={outgoingTheme.renderer} {...{ rehydrate: false }}>
<ThemeProvider theme={outgoingTheme}>{children}</ThemeProvider>
Expand All @@ -132,6 +134,14 @@ class Provider extends React.Component<ProviderProps, any> {
/>
)
}

renderStaticStylesOnce = (mergedTheme: ThemePrepared) => {
const { staticStyles } = this.props.theme
if (!this.staticStylesRendered && staticStyles) {
this.renderStaticStyles(mergedTheme)
this.staticStylesRendered = true
}
}
}

export default Provider
44 changes: 44 additions & 0 deletions test/specs/components/Provider/Provider-test.tsx
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand All @@ -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(
<Provider theme={{ siteVariables: { brand: 'blue', background: 'red' } }}>
<Provider
theme={{
siteVariables: { brand: 'yellow', gray: '#868686' },
staticStyles: [staticStyle],
}}
>
<span />
</Provider>
</Provider>,
)

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(
<Provider theme={{ staticStyles: [firstStaticStyle] }}>
<span />
</Provider>,
)
providerInstance.setProps({ theme: { staticStyles: [secondStaticStyle] } })

expect(firstStaticStyle).toHaveBeenCalledTimes(1)
expect(secondStaticStyle).not.toHaveBeenCalled()
})
})
})