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

fix(doc): Use correct LTR/RTL Fela renderer in renderComponent #34

Merged
merged 4 commits into from
Aug 1, 2018
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

### Documentation
- Improve UX for "knobs" form on component examples @levithomason ([#20](https://github.com/stardust-ui/react/pull/20))
- Use correct styles in RTL component preview @miroslavstastny ([#34](https://github.com/stardust-ui/react/pull/34))

### Fixes
- Replaced Header `subheader` with `description` and fixed it to render well-formed HTML @mnajdova ([#17](https://github.com/stardust-ui/react/pull/17))
Expand Down
32 changes: 22 additions & 10 deletions src/components/Provider/Provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,22 +36,26 @@ class Provider extends Component<any, any> {

static Consumer = ProviderConsumer

renderStaticStyles = felaRenderer => {
renderStaticStyles = () => {
// RTL WARNING
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it would be a good idea to include reference on docs where it is stated that

no further processing with plugins will be done for static styles

http://fela.js.org/docs/advanced/StaticStyle.html

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, link added.

// This function sets static styles which are global and renderer agnostic
// With current implementation, static styles cannot differ between LTR and RTL

const { siteVariables, staticStyles } = this.props

if (!staticStyles) return

const renderObject = object => {
_.forEach(object, (style, selector) => {
felaRenderer.renderStatic(style, selector)
felaLtrRenderer.renderStatic(style, selector)
})
}

const staticStylesArr = [].concat(staticStyles)

staticStylesArr.forEach(staticStyle => {
if (_.isString(staticStyle)) {
felaRenderer.renderStatic(staticStyle)
felaLtrRenderer.renderStatic(staticStyle)
} else if (_.isPlainObject(staticStyle)) {
renderObject(staticStyle)
} else if (_.isFunction(staticStyle)) {
Expand All @@ -64,7 +68,11 @@ class Provider extends Component<any, any> {
})
}

renderFontFaces = felaRenderer => {
renderFontFaces = () => {
// RTL WARNING
// This function sets static styles which are global and renderer agnostic
// With current implementation, static styles cannot differ between LTR and RTL

const { siteVariables, fontFaces } = this.props

if (!fontFaces) return
Expand All @@ -73,7 +81,7 @@ class Provider extends Component<any, any> {
if (!_.isPlainObject(font)) {
throw new Error(`fontFaces must be objects, got: ${typeof font}`)
}
felaRenderer.renderFont(font.name, font.path, font.style)
felaLtrRenderer.renderFont(font.name, font.path, font.style)
}

const fontFaceArr = [].concat(_.isFunction(fontFaces) ? fontFaces(siteVariables) : fontFaces)
Expand All @@ -84,17 +92,21 @@ class Provider extends Component<any, any> {
}

componentDidMount() {
const felaRenderer = this.props.rtl ? felaRtlRenderer : felaLtrRenderer
this.renderStaticStyles(felaRenderer)
this.renderFontFaces(felaRenderer)
this.renderStaticStyles()
this.renderFontFaces()
}

render() {
const { componentVariables, siteVariables, children, rtl } = this.props
const renderer = rtl ? felaRtlRenderer : felaLtrRenderer

// ensure we don't assign `undefined` values to the theme context
// they will override values down stream
const theme: any = { rtl: !!rtl }
const theme: any = {
renderer,
rtl: !!rtl,
}

if (siteVariables) {
theme.siteVariables = siteVariables
}
Expand All @@ -103,7 +115,7 @@ class Provider extends Component<any, any> {
}

return (
<RendererProvider renderer={this.props.rtl ? felaRtlRenderer : felaLtrRenderer}>
<RendererProvider renderer={renderer}>
<ThemeProvider theme={theme}>{children}</ThemeProvider>
</RendererProvider>
)
Expand Down
10 changes: 7 additions & 3 deletions src/lib/getClasses.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import renderer from './felaRenderer'

export interface IClasses {
[key: string]: string
}
Expand All @@ -11,7 +9,13 @@ export interface IClasses {
* @param theme
* @returns {{}}
*/
const getClasses = (props, rules, variables: any = () => {}, theme: any = {}): IClasses => {
const getClasses = (
renderer,
props,
rules,
variables: any = () => {},
theme: any = {},
): IClasses => {
const { renderRule } = renderer
const ruleProps = {
props,
Expand Down
4 changes: 2 additions & 2 deletions src/lib/renderComponent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const renderComponent = <P extends {}>(
return (
<FelaTheme
render={theme => {
const { siteVariables = {}, componentVariables = {} } = theme
const { siteVariables = {}, componentVariables = {}, renderer } = theme

const ElementType = getElementType({ defaultProps }, props)
const rest = getUnhandledProps({ handledProps }, props)
Expand All @@ -45,7 +45,7 @@ const renderComponent = <P extends {}>(
const mergedVariables = () =>
Object.assign({}, variablesFromFile, variablesFromTheme, variablesFromProp)

const classes = getClasses(props, rules, mergedVariables, theme)
const classes = getClasses(renderer, props, rules, mergedVariables, theme)
classes.root = cx(className, classes.root, props.className)

const config: IRenderResultConfig<P> = { ElementType, rest, classes }
Expand Down
18 changes: 15 additions & 3 deletions test/specs/commonTests/isConformant.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
import _ from 'lodash'
import React from 'react'
import { shallow, mount, render } from 'enzyme'
import { shallow, mount as enzymeMount, render } from 'enzyme'
import ReactDOMServer from 'react-dom/server'
import { ThemeProvider } from 'react-fela'

import { assertBodyContains, consoleUtil, syntheticEvent } from 'test/utils'
import helpers from './commonHelpers'

import * as stardust from 'src/'
import { felaRenderer } from 'src/lib'

const mount = (node, options?) => {
return enzymeMount(
<ThemeProvider theme={{ renderer: felaRenderer }}>{node}</ThemeProvider>,
options,
)
}

/**
* Assert Component conforms to guidelines that are applicable to all components.
Expand All @@ -24,7 +33,10 @@ export default (Component, options: any = {}) => {

// This is added because of the FelaTheme wrapper and the component itself, because it is mounted
const getComponent = wrapper => {
return wrapper.childAt(0).childAt(0)
return wrapper
.childAt(0)
.childAt(0)
.childAt(0)
}

// make sure components are properly exported
Expand Down Expand Up @@ -260,7 +272,7 @@ export default (Component, options: any = {}) => {
'data-simulate-event-here': true,
}

const component = mount(<Component {...props} />)
const component = mount(<Component {...props} />).childAt(0)

const eventTarget = eventTargets[listenerName]
? component
Expand Down