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

Commit e5052b8

Browse files
authored
fix(factories): reverted changed for not passing props to elements in the factories (#759)
* -reverted changed for not passing props to react elements in the factories * -updated shorthand props documentation * -updated changelog * -updated shorthand props docs * -updated shorthand props docs * -updated shorthand props docs * -updated chat divider styles & example
1 parent 0f1cff7 commit e5052b8

File tree

6 files changed

+46
-28
lines changed

6 files changed

+46
-28
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
2424
- Fix teams theme `Status` and `Chat.Message` styles ([#747](https://github.com/stardust-ui/react/pull/747))
2525
- Fix `Popup` - do not stop event propagation when pressing Esc on trigger element @sophieH29 ([#750](https://github.com/stardust-ui/react/pull/750))
2626
- Fix alignment of `Layout`'s `main` area @kuzhelov ([#752](https://github.com/stardust-ui/react/pull/752))
27+
- Forwarding props for `createShorthand` calls if the value is a React element @mnajdova ([#759](https://github.com/stardust-ui/react/pull/759))
2728

2829
### Features
2930
- Rename `Slot` component to `Box` and export it @Bugaa92 ([#713](https://github.com/stardust-ui/react/pull/713))

docs/src/examples/components/Chat/Types/ChatExample.shorthand.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ const items = [
5353
key: 'message-id-5',
5454
},
5555
{
56-
message: <Divider content="Today" color="primary" important />,
56+
children: <Divider content="Today" color="primary" important />,
5757
key: 'message-id-6',
5858
},
5959
{

docs/src/prototypes/chatPane/index.tsx

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
import * as React from 'react'
2-
import { Provider } from '@stardust-ui/react'
2+
import { Provider, Divider } from '@stardust-ui/react'
33

44
import ChatPaneLayout from './chatPaneLayout'
55
import { getChatMock } from './services'
6+
import { pxToRem } from '../../../../src/lib'
67

78
const chatMock = getChatMock({ msgCount: 40, userCount: 6 })
9+
const dividerSelector = `& .${Divider.className}`
810

911
export default () => (
1012
<Provider
@@ -14,6 +16,14 @@ export default () => (
1416
start: { display: 'block' },
1517
end: { display: 'block' },
1618
},
19+
ChatItem: {
20+
root: {
21+
[dividerSelector]: {
22+
marginLeft: `-${pxToRem(40)}`,
23+
marginRight: `-${pxToRem(40)}`,
24+
},
25+
},
26+
},
1727
},
1828
}}
1929
>

docs/src/views/ShorthandProps.tsx

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -86,14 +86,13 @@ const ShorthandProps = props => (
8686
<blockquote>
8787
<strong>
8888
There is a very important caveat here, though: whenever React Element is directly used as a
89-
shorthand value, it becomes client's responsibility to handle some aspects that Stardust was
90-
originally responsible for (such as {code('styles')} or {code('accessibility')}).
89+
shorthand value, all props that Stardust has created for the slot's Component will be spread
90+
on the passed element. This means that provided element should be able to handle Stardust
91+
props - while this requirement is satisfied for all Stardust components, you should be aware
92+
of that when either HTML or any third-party elements are provided.
9193
</strong>{' '}
92-
This is because, in contrast to other forms of shorthand values, Stardust-evaluated props
93-
cannot be safely passed to the element which type is, generally, doesn't allow Stardust to
94-
make any prior assumptions about. Due to this limitation, you should strive to use other
95-
options for shorthand values whenever is possible - for instance, this is how previous example
96-
can be rewritten:
94+
Due to this limitation, you should strive to use other options for shorthand values whenever
95+
is possible - for instance, this is how previous example can be rewritten:
9796
</blockquote>
9897
{codeExample([`<Button icon={{ as: 'i', className: 'my-icon' }} />`])}
9998

src/lib/factories.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -129,18 +129,16 @@ function createShorthandFromValue(
129129
)
130130
}
131131

132-
// return value 'as is' if it a ReactElement
133-
if (valIsReactElement) {
134-
return value as React.ReactElement<Props>
135-
}
136-
137132
// ----------------------------------------
138133
// Build up props
139134
// ----------------------------------------
140135
const { defaultProps = {} } = options
141136

142137
// User's props
143-
const usersProps = valIsPropsObject ? (value as Props) : {}
138+
const usersProps =
139+
(valIsReactElement && (value as React.ReactElement<Props>).props) ||
140+
(valIsPropsObject && (value as Props)) ||
141+
{}
144142

145143
// Override props
146144
let { overrideProps } = options
@@ -199,6 +197,9 @@ function createShorthandFromValue(
199197
return render(Component, props)
200198
}
201199

200+
// Clone ReactElements
201+
if (valIsReactElement) return React.cloneElement(value as React.ReactElement<Props>, props)
202+
202203
// Create ReactElements from built up props
203204
if (valIsPrimitive || valIsPropsObject) return React.createElement(Component, props)
204205

test/specs/lib/factories-test.tsx

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,16 @@ describe('factories', () => {
492492
testCreateShorthand({ overrideProps, value: testValue }, overrideProps())
493493
})
494494

495+
test("is called with the user's element's and default props", () => {
496+
const defaultProps = { 'data-some': 'defaults' }
497+
const overrideProps = jest.fn(() => ({}))
498+
const userProps = { 'data-user': 'props' }
499+
const value = <div {...userProps} />
500+
501+
shallow(getShorthand({ defaultProps, overrideProps, value }))
502+
expect(overrideProps).toHaveBeenCalledWith({ ...defaultProps, ...userProps })
503+
})
504+
495505
test("is called with the user's props object", () => {
496506
const defaultProps = { 'data-some': 'defaults' }
497507
const overrideProps = jest.fn(() => ({}))
@@ -524,21 +534,18 @@ describe('factories', () => {
524534

525535
describe('from an element', () => {
526536
itReturnsAValidElement(<div />)
537+
itAppliesDefaultProps(<div />)
527538
itDoesNotIncludePropsFromMappedProp(<div />)
539+
itMergesClassNames('element', 'user', { value: <div className="user" /> })
528540
itAppliesProps('element', { foo: 'foo' }, { value: <div {...{ foo: 'foo' } as any} /> })
529-
530-
test('forwards original element "as is"', () => {
531-
testCreateShorthand(
532-
{
533-
Component: 'p',
534-
value: (
535-
<span {...{ commonProp: 'originalElement', originalElementProp: true } as any} />
536-
),
537-
defaultProps: { commonProp: 'default', defaultProp: true },
538-
overrideProps: { commonProp: 'override', overrideProp: true },
539-
},
540-
{ commonProp: 'originalElement', originalElementProp: true },
541-
)
541+
itOverridesDefaultProps(
542+
'element',
543+
{ some: 'defaults', overridden: false },
544+
{ some: 'defaults', overridden: true },
545+
{ value: <div {...{ overridden: true } as any} /> },
546+
)
547+
itOverridesDefaultPropsWithFalseyProps('element', {
548+
value: <div {...{ undef: undefined, nil: null, zero: 0, empty: '' } as any} />,
542549
})
543550
})
544551

0 commit comments

Comments
 (0)