diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e76b01782..993f743c4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Add `https` protocol to all urls used in the scripts and stylesheets in index.ejs @mnajdova ([#571](https://github.com/stardust-ui/react/pull/571)) - Fix support for fallback values in styles (`color: ['#ccc', 'rgba(0, 0, 0, 0.5)']`) @miroslavstastny ([#573](https://github.com/stardust-ui/react/pull/573)) - Fix styles for RTL mode of doc site component examples @kuzhelov ([#579](https://github.com/stardust-ui/react/pull/579)) +- Prevent blind props forwarding for `createShorthand` calls if the value is a React element and remove manual check for `Input` `wrapper` @Bugaa92 ([#496](https://github.com/stardust-ui/react/pull/496)) ### Features - `Ref` components uses `forwardRef` API by default @layershifter ([#491](https://github.com/stardust-ui/react/pull/491)) diff --git a/docs/src/examples/components/Input/Variations/InputExampleInputSlot.shorthand.tsx b/docs/src/examples/components/Input/Variations/InputExampleInputSlot.shorthand.tsx index 50589dbe28..fb461ab694 100644 --- a/docs/src/examples/components/Input/Variations/InputExampleInputSlot.shorthand.tsx +++ b/docs/src/examples/components/Input/Variations/InputExampleInputSlot.shorthand.tsx @@ -22,20 +22,6 @@ const InputExampleInputSlot = () => ( styles: inputStyles, }} /> - - - - } - /> ) diff --git a/docs/src/examples/components/Input/Variations/InputExampleWrapperSlot.shorthand.tsx b/docs/src/examples/components/Input/Variations/InputExampleWrapperSlot.shorthand.tsx index d6a65cda35..51d8551e72 100644 --- a/docs/src/examples/components/Input/Variations/InputExampleWrapperSlot.shorthand.tsx +++ b/docs/src/examples/components/Input/Variations/InputExampleWrapperSlot.shorthand.tsx @@ -23,22 +23,6 @@ const InputExampleWrapperSlot = () => ( styles: { padding: '5px', backgroundColor: 'red' }, }} /> - - - } - /> - - - } - /> ) diff --git a/docs/src/examples/components/RadioGroup/Item/RadioGroupItemExample.shorthand.tsx b/docs/src/examples/components/RadioGroup/Item/RadioGroupItemExample.shorthand.tsx index 25ab8310f9..1f4cb4ee3c 100644 --- a/docs/src/examples/components/RadioGroup/Item/RadioGroupItemExample.shorthand.tsx +++ b/docs/src/examples/components/RadioGroup/Item/RadioGroupItemExample.shorthand.tsx @@ -8,8 +8,8 @@ const handleChange = () => { const RadioGroupItemExample = () => ( , - , + { key: '1', label: 'Make your choice', value: '1', checkedChanged: handleChange }, + { key: '2', label: 'Another option', value: '2', checkedChanged: handleChange }, ]} /> ) diff --git a/docs/src/examples/components/RadioGroup/Item/RadioGroupItemExampleChecked.shorthand.tsx b/docs/src/examples/components/RadioGroup/Item/RadioGroupItemExampleChecked.shorthand.tsx index 01c1f5c349..d00e09bb50 100644 --- a/docs/src/examples/components/RadioGroup/Item/RadioGroupItemExampleChecked.shorthand.tsx +++ b/docs/src/examples/components/RadioGroup/Item/RadioGroupItemExampleChecked.shorthand.tsx @@ -4,7 +4,10 @@ import { RadioGroup } from '@stardust-ui/react' const RadioGroupItemExampleCheckedShorthand = () => ( ]} + items={[ + { key: '1', label: 'This radio comes pre-checked', value: '1' }, + { key: '2', label: 'This radio is not pre-checked', value: '2' }, + ]} /> ) diff --git a/docs/src/examples/components/RadioGroup/Item/RadioGroupItemExampleDisabled.shorthand.tsx b/docs/src/examples/components/RadioGroup/Item/RadioGroupItemExampleDisabled.shorthand.tsx index ba579aa7a7..135f9d4621 100644 --- a/docs/src/examples/components/RadioGroup/Item/RadioGroupItemExampleDisabled.shorthand.tsx +++ b/docs/src/examples/components/RadioGroup/Item/RadioGroupItemExampleDisabled.shorthand.tsx @@ -4,8 +4,8 @@ import { RadioGroup } from '@stardust-ui/react' const RadioGroupItemExampleDisabledShorthand = () => ( , - , + { key: '1', label: 'Disabled', value: '1', disabled: true }, + { key: '2', label: 'Enabled', value: '2' }, ]} /> ) diff --git a/docs/src/examples/components/RadioGroup/Types/RadioGroupColorPickerExample.shorthand.tsx b/docs/src/examples/components/RadioGroup/Types/RadioGroupColorPickerExample.shorthand.tsx index a2a6b936d0..29132a0237 100644 --- a/docs/src/examples/components/RadioGroup/Types/RadioGroupColorPickerExample.shorthand.tsx +++ b/docs/src/examples/components/RadioGroup/Types/RadioGroupColorPickerExample.shorthand.tsx @@ -2,9 +2,33 @@ import React from 'react' import { Divider, RadioGroup } from '@stardust-ui/react' class RadioGroupColorPickerExample extends React.Component { - createIcon = value => { + state = { selectedValue: '' } + + render() { + const { selectedValue } = this.state + return ( +
+ The selected value is: {selectedValue} + + ({ + key: color, + value: color, + name: color, + 'aria-label': color, + icon: this.createIcon(color), + }))} + checkedValueChanged={(e, props) => this.setState({ selectedValue: props.value })} + /> +
+ ) + } + + createIcon(value) { const { selectedValue } = this.state const isSelected = selectedValue === value + return { variables: { backgroundColor: value, @@ -22,37 +46,6 @@ class RadioGroupColorPickerExample extends React.Component { }, } } - - items = () => { - const colors = ['pink', 'blue', 'green', 'red', 'orange'] - return colors.map(color => ( - - )) - } - - state = { selectedValue: '' } - handleChange = (e, props) => { - this.setState({ selectedValue: props.value }) - } - render() { - const { selectedValue } = this.state - return ( -
- The selected value is: {selectedValue} - - -
- ) - } } + export default RadioGroupColorPickerExample diff --git a/src/components/Input/Input.tsx b/src/components/Input/Input.tsx index b403548f95..c9f9898b66 100644 --- a/src/components/Input/Input.tsx +++ b/src/components/Input/Input.tsx @@ -146,13 +146,8 @@ class Input extends AutoControlledComponent, InputState> })} ), + styles: styles.root, ...rest, - - // do not pass Stardust 'styles' prop - // in case if React Element was used to define 'wrapper' - ...(!React.isValidElement(wrapper) && { - styles: styles.root, - }), }, overrideProps: { as: (wrapper && (wrapper as any).as) || ElementType, diff --git a/src/lib/factories.tsx b/src/lib/factories.tsx index 2684c19ef0..c7b18e6552 100644 --- a/src/lib/factories.tsx +++ b/src/lib/factories.tsx @@ -126,16 +126,18 @@ function createShorthandFromValue( ) } + // return value 'as is' if it a ReactElement + if (valIsReactElement) { + return value as React.ReactElement + } + // ---------------------------------------- // Build up props // ---------------------------------------- const { defaultProps = {} } = options // User's props - const usersProps = - (valIsReactElement && (value as React.ReactElement).props) || - (valIsPropsObject && (value as Props)) || - {} + const usersProps = valIsPropsObject ? (value as Props) : {} // Override props let { overrideProps } = options @@ -191,9 +193,6 @@ function createShorthandFromValue( return render(Component, props) } - // Clone ReactElements - if (valIsReactElement) return React.cloneElement(value as React.ReactElement, props) - // Create ReactElements from built up props if (valIsPrimitive || valIsPropsObject) return diff --git a/test/specs/commonTests/implementsWrapperProp.tsx b/test/specs/commonTests/implementsWrapperProp.tsx index 9aaaa34f9a..a2feb9a964 100644 --- a/test/specs/commonTests/implementsWrapperProp.tsx +++ b/test/specs/commonTests/implementsWrapperProp.tsx @@ -7,14 +7,14 @@ import { ShorthandValue } from 'utils' export interface ImplementsWrapperPropOptions { wrapppedComponentSelector: any - wrappperComponentSelector?: any + WrapperComponent?: any } const implementsWrapperProp =

( Component: React.ReactType

, options: ImplementsWrapperPropOptions, ) => { - const { wrapppedComponentSelector, wrappperComponentSelector = Slot.defaultProps.as } = options + const { wrapppedComponentSelector, WrapperComponent = Slot } = options const wrapperTests = (wrapper: ReactWrapper) => { expect(wrapper.length).toBeGreaterThan(0) @@ -23,11 +23,7 @@ const implementsWrapperProp =

( describe('"wrapper" prop', () => { it('wraps the component by default', () => { - wrapperTests(mount().find(wrappperComponentSelector)) - }) - - it('wraps the component with a custom element', () => { - wrapperTests(mount(} />).find('span')) + wrapperTests(mount().find(WrapperComponent)) }) it('wraps the component with a custom element using "as" prop', () => { diff --git a/test/specs/components/RadioGroup/RadioGroup-test.tsx b/test/specs/components/RadioGroup/RadioGroup-test.tsx index b875c59326..bfebdd896d 100644 --- a/test/specs/components/RadioGroup/RadioGroup-test.tsx +++ b/test/specs/components/RadioGroup/RadioGroup-test.tsx @@ -60,7 +60,7 @@ describe('RadioGroup', () => { }) }) - const itemsTest = getItems => { + const itemsTest = (getItems: Function, isShorthandApiTest: boolean = true) => { it('renders children', () => { const items = mountWithProvider().find('RadioGroupItem') @@ -101,31 +101,33 @@ describe('RadioGroup', () => { }) }) - describe('click event handler', () => { - it('should set the value when item is clicked', () => { - const checkedValueChanged = jest.fn() - const wrapper = mountWithProvider( - , - ) - const radioGroupItems = wrapper.find('RadioGroupItem') + if (isShorthandApiTest) { + describe('click event handler', () => { + it('should set "checked" when item is clicked', () => { + const checkedValueChanged = jest.fn() + const wrapper = mountWithProvider( + , + ) + const radioGroupItems = wrapper.find('RadioGroupItem') - radioGroupItems - .at(1) - .find('div') - .first() - .simulate('click') + radioGroupItems + .at(1) + .find('div') + .first() + .simulate('click') - const updatedItems = wrapper.find('RadioGroupItem') + const updatedItems = wrapper.find('RadioGroupItem') - expect(updatedItems.at(0).props().checked).toBe(false) - expect(updatedItems.at(1).props().checked).toBe(true) + expect(updatedItems.at(0).props().checked).toBe(false) + expect(updatedItems.at(1).props().checked).toBe(true) - expect(checkedValueChanged).toHaveBeenCalledWith( - expect.anything(), - expect.objectContaining({ value: 'test-value2' }), - ) + expect(checkedValueChanged).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ value: 'test-value2' }), + ) + }) }) - }) + } it('should not call checkedValueChanged when index did not change', () => { const checkedValueChanged = jest.fn() @@ -147,21 +149,23 @@ describe('RadioGroup', () => { expect(checkedValueChanged).not.toHaveBeenCalled() }) - it('should not set the value when disabled item is clicked', () => { - const wrapper = mountWithProvider() - const radioGroupItems = wrapper.find('RadioGroupItem') + if (isShorthandApiTest) { + it('should not set "checked" when disabled item is clicked', () => { + const wrapper = mountWithProvider() + const radioGroupItems = wrapper.find('RadioGroupItem') - radioGroupItems - .at(1) - .find('div') - .first() - .simulate('click') + radioGroupItems + .at(1) + .find('div') + .first() + .simulate('click') - const updatedItems = wrapper.find('RadioGroupItem') + const updatedItems = wrapper.find('RadioGroupItem') - expect(updatedItems.at(0).props().checked).toBe(false) - expect(updatedItems.at(1).props().checked).toBe(false) - }) + expect(updatedItems.at(0).props().checked).toBe(false) + expect(updatedItems.at(1).props().checked).toBe(false) + }) + } describe('keyDown event handler', () => { const testKeyDown = (testName, items, initialValue, keyCode, expectedValue) => { @@ -244,6 +248,6 @@ describe('RadioGroup', () => { }) } - itemsTest(getChildrenItems) + itemsTest(getChildrenItems, false) }) }) diff --git a/test/specs/lib/factories-test.tsx b/test/specs/lib/factories-test.tsx index 5b3b22461a..279bce263c 100644 --- a/test/specs/lib/factories-test.tsx +++ b/test/specs/lib/factories-test.tsx @@ -10,7 +10,7 @@ import callable from '../../../src/lib/callable' // Utils // ---------------------------------------- -type GetShorthandArgs = { +type ShorthandConfig = { Component?: React.ReactType defaultProps?: Props mappedProp?: string @@ -31,7 +31,7 @@ const getShorthand = ({ generateKey, value, render, -}: GetShorthandArgs) => +}: ShorthandConfig) => createShorthand(Component, mappedProp, value, { defaultProps, overrideProps, @@ -42,7 +42,7 @@ const getShorthand = ({ const isValuePrimitive = (value: ShorthandValue) => typeof value === 'string' || typeof value === 'number' -const testCreateShorthand = (shorthandArgs: GetShorthandArgs, expectedResult: ObjectOf) => +const testCreateShorthand = (shorthandArgs: ShorthandConfig, expectedResult: ObjectOf) => expect(shallow(getShorthand(shorthandArgs)).props()).toEqual(expectedResult) // ---------------------------------------- @@ -111,7 +111,11 @@ const itMergesClassNames = ( }) } -const itAppliesProps = (propsSource, expectedProps, shorthandConfig) => { +const itAppliesProps = ( + propsSource: string, + expectedProps: Props, + shorthandConfig: ShorthandConfig, +) => { test(`applies props from the ${propsSource} props`, () => { testCreateShorthand(shorthandConfig, expectedProps) }) @@ -482,16 +486,6 @@ describe('factories', () => { testCreateShorthand({ overrideProps, value: testValue }, overrideProps()) }) - test("is called with the user's element's and default props", () => { - const defaultProps = { 'data-some': 'defaults' } - const overrideProps = jest.fn(() => ({})) - const userProps = { 'data-user': 'props' } - const value =

- - shallow(getShorthand({ defaultProps, overrideProps, value })) - expect(overrideProps).toHaveBeenCalledWith({ ...defaultProps, ...userProps }) - }) - test("is called with the user's props object", () => { const defaultProps = { 'data-some': 'defaults' } const overrideProps = jest.fn(() => ({})) @@ -524,18 +518,21 @@ describe('factories', () => { describe('from an element', () => { itReturnsAValidElement(
) - itAppliesDefaultProps(
) itDoesNotIncludePropsFromMappedProp(
) - itMergesClassNames('element', 'user', { value:
}) itAppliesProps('element', { foo: 'foo' }, { value:
}) - itOverridesDefaultProps( - 'element', - { some: 'defaults', overridden: false }, - { some: 'defaults', overridden: true }, - { value:
}, - ) - itOverridesDefaultPropsWithFalseyProps('element', { - value:
, + + test('forwards original element "as is"', () => { + testCreateShorthand( + { + Component: 'p', + value: ( + + ), + defaultProps: { commonProp: 'default', defaultProp: true }, + overrideProps: { commonProp: 'override', overrideProp: true }, + }, + { commonProp: 'originalElement', originalElementProp: true }, + ) }) })