From 589b7ae6f9dd6b3e4f50b282c30c3b7280018384 Mon Sep 17 00:00:00 2001 From: Satyajit Sahoo Date: Sat, 12 Aug 2017 18:00:25 +0200 Subject: [PATCH] Minor refactor --- docs/API.md | 2 - src/PaletteProvider.js | 82 +++++++++++++-------------- src/__tests__/PaletteProvider.test.js | 45 +++++++-------- src/__tests__/withPalette.test.js | 13 ++--- src/withPalette.js | 5 +- 5 files changed, 64 insertions(+), 83 deletions(-) diff --git a/docs/API.md b/docs/API.md index d8baddc..a6ce215 100644 --- a/docs/API.md +++ b/docs/API.md @@ -73,8 +73,6 @@ The concept is very similar to `Provider` component from `react-redux`. * `onError?: (error: Error) => void` (optional) - Error handler, called if the palette failed to create. -* `onInit?: () => void` - (optional) - Init handler, called when the `MaterialPaletteProvider` is just about to start creating the palette. - * `onFinish?: (palette: PaletteInstance) => void` - (optional) - Finish handler, called when the palette is created, but before it gets propagated to _connected_ components - use it, if you want to mutate the palette instance. If some profiles are not available for the provided image, the defaults will apply, taking precedence the ones you passed to the component as `this.props.defaults`. * `children: React$Element<*>`, - (__required__) - Children elements - the rest of your app's component tree. diff --git a/src/PaletteProvider.js b/src/PaletteProvider.js index 511ddfc..17cb799 100644 --- a/src/PaletteProvider.js +++ b/src/PaletteProvider.js @@ -38,10 +38,6 @@ type Props = { * Error handler, called when palette generation fails */ onError?: (error: Error) => void, - /** - * Initialization handler, called right before generation the palette - */ - onInit?: () => void, /** * Finish handler, called right after the palette is generated */ @@ -54,23 +50,13 @@ type Props = { /** * Render LoaderComponent when the palette is being created */ - LoaderComponent?: - | React$Component<*, *, *> - | ((...args: *) => React$Element<*>), + LoaderComponent?: ReactClass<*> | ((...args: Array<*>) => React.Element<*>), }; type State = { palette: ?PaletteInstance, }; -function execIfFunction(possibleFunction: mixed, ...args: *): boolean { - if (typeof possibleFunction === 'function') { - possibleFunction(...args); - return true; - } - return false; -} - /** * Provides broadcast for material palette instance via context. * Passes `subscribe` method via context, which `withPalette` can call @@ -78,18 +64,14 @@ function execIfFunction(possibleFunction: mixed, ...args: *): boolean { */ export default class MaterialPaletteProvider extends Component { - state: State; - - constructor(props: Props) { - super(props); - this.state = { - palette: null, - }; - } static childContextTypes = { [KEY]: PropTypes.func.isRequired, }; + state: State = { + palette: null, + }; + eventEmitter = createEventEmitter(null); getChildContext() { @@ -144,35 +126,47 @@ export default class MaterialPaletteProvider if (this.props.defaults) { validateDefaults(this.props.defaults); } - execIfFunction(this.props.onInit); - createMaterialPalette(this.props.image, this.props.options) - .then((palette: PaletteInstance) => { + + this._createPalette(); + } + + _createPalette = () => { + const { image, options, onFinish, onError } = this.props; + + createMaterialPalette(image, options).then( + palette => { const paletteWithDefaults = this._mergeWithDefaults(palette); - execIfFunction(this.props.onFinish, paletteWithDefaults); - if (!this.props.forceRender) { - this.setState({ palette: paletteWithDefaults }); - } + + if (onFinish) onFinish(paletteWithDefaults); + this.eventEmitter.publish({ palette: paletteWithDefaults, }); - }) - .catch((error: Error) => { - const isCalled = execIfFunction(this.props.onError, error); - if (!isCalled) { - const enhancedError = error; - enhancedError.message = `Uncaught MaterialPaletteProvider exception: ${enhancedError.message}`; - throw enhancedError; + this.setState({ palette: paletteWithDefaults }); + }, + error => { + if (onError) { + onError(error); + } else { + error.message = `Uncaught MaterialPaletteProvider exception: ${error.message}`; // eslint-disable-line no-param-reassign + throw error; } - }); - } + }, + ); + }; render() { - if (!this.state.palette && this.props.LoaderComponent) { - return ; - } else if (!this.state.palette && !this.props.forceRender) { - return null; + const { forceRender, LoaderComponent, children } = this.props; + const shouldRender = this.state.palette || forceRender; + + if (LoaderComponent && !this.state.palette) { + return ; + } + + if (shouldRender) { + return React.Children.only(children); } - return React.Children.only(this.props.children); + return null; } } diff --git a/src/__tests__/PaletteProvider.test.js b/src/__tests__/PaletteProvider.test.js index c1c2999..8859795 100644 --- a/src/__tests__/PaletteProvider.test.js +++ b/src/__tests__/PaletteProvider.test.js @@ -30,7 +30,7 @@ describe('PaletteProvider', () => { createMaterialPalette.mockReset(); }); - it('should create palette and call `onInit` and `onFinish` handlers', done => { + it('should create palette and call `onFinish` handler when done', done => { createMaterialPalette.mockImplementation(() => Promise.resolve({ vibrant: null })); @@ -102,33 +102,26 @@ describe('PaletteProvider', () => { ); }); - it('should throw error if `onError` was not passed and palette creation fails', () => - new Promise(resolve => { - function checkErrorAndFinish(error) { - expect(error.message).toMatch(/MaterialPaletteProvider.*test/); - resolve(); - } - - createMaterialPalette.mockImplementation(() => ({ - then() { - return this; - }, - catch(errorHandler) { - try { - errorHandler(new Error('test')); - } catch (error) { - checkErrorAndFinish(error); - } - }, - })); - - render( - - Test - , - ); + it('should throw error if `onError` was not passed and palette creation fails', () => { + createMaterialPalette.mockImplementation(() => ({ + then(success, failure) { + try { + failure(new Error('test')); + } catch (error) { + expect(error.message).toBe( + 'Uncaught MaterialPaletteProvider exception: test', + ); + } + }, })); + render( + + Test + , + ); + }); + it('should render children if `forceRender` is true when creating palette', done => { createMaterialPalette.mockImplementation( () => diff --git a/src/__tests__/withPalette.test.js b/src/__tests__/withPalette.test.js index de6d284..1d1428d 100644 --- a/src/__tests__/withPalette.test.js +++ b/src/__tests__/withPalette.test.js @@ -1,5 +1,6 @@ import React from 'react'; import { shallow } from 'enzyme'; +import { StyleSheet } from 'react-native'; import withPalette from '../withPalette'; import { KEY } from '../PaletteProvider'; @@ -159,19 +160,17 @@ describe('withPalette', () => { expect(palette).toEqual({ lightVibrant: { ...localDefaults.lightVibrant, population: 0 }, }); - expect(style).toEqual([{ fontSize: '14px' }, {}]); + expect(StyleSheet.flatten(style)).toEqual({ fontSize: '14px' }); } function onSecondRender(palette, style) { expect(palette).toEqual({ ...paletteMock, lightVibrant: { population: 0, ...localDefaults.lightVibrant }, }); - expect(style).toEqual([ - { fontSize: '14px' }, - { - color: localDefaults.lightVibrant.color, - }, - ]); + expect(StyleSheet.flatten(style)).toEqual({ + fontSize: '14px', + color: localDefaults.lightVibrant.color, + }); } const PaletteTest = withPalette( diff --git a/src/withPalette.js b/src/withPalette.js index 6a4e5c1..7f4febf 100644 --- a/src/withPalette.js +++ b/src/withPalette.js @@ -104,10 +104,7 @@ export default function withMaterialPalette( return ( );