-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Breaking Changes: Add component augmentation #414
Conversation
Current coverage is 96.17% (diff: 95.54%)@@ master #414 diff @@
==========================================
Files 91 84 -7
Lines 1175 1255 +80
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 1118 1207 +89
+ Misses 57 48 -9
Partials 0 0
|
5422924
to
e7ec341
Compare
d2598e0
to
007b623
Compare
// ---------------------------------------- | ||
// user defined element type | ||
|
||
if (props.as && props.as !== defaultProps.as) return props.as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this just always return props.as
if it exists?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me go back and figure out why I did this, could be cruft. Used to have defaultAs
prop here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, this is still required. The element type is chosen from this priority:
- User defined an
as
prop - getElementType() typeMap arg mapped a prop to an element type:
{ onClick: 'a' }
- Inferred type: `href => a'
- defaultProp.as
div
When there is a defaultProp, the props.as
will be set. However, this is not the user's as
value. It is the component default. If we remove the props.as !== defaultProps.as
comparison, then the defaultProp.as is always used as the first priority since it is copied to props.as
.
So, we only want to use the props.as
if the user defined one (i.e. it does not equal the default).
Image.js
- default -
img
- maps -
wrapped => div
- infers -
href => a
If the props.as is used, it will always be an img
. Even if the user uses wrapped
, which should be a div or href
which should wrap the image in an a
tag.
render() {
const ElementType = getElementType(Image, props, {
wrapped: 'div',
})
if (ElementType === 'img') {
return <ElementType {...rootProps} {...imgTagProps} />
}
return (
<ElementType {...rootProps} href={href}>
<img {...imgTagProps} />
</ElementType>
)
}
Image.defaultProps = {
as: 'img',
}
👻 |
007b623
to
511860f
Compare
Released in |
Fixes #403. This PR will add component augmentation.
Add Dropdown as Button augmentation examplerelated but beyond the scope of this PRInvestigate solving feat(Dropdown): support label prop #291related but beyond the scope of this PRTesting Changes
TechnologyAdvice/stardust@415c870 We have well over 2k tests. This PR updated the watcher to only run changed tests. If you change a non test file, all tests will re-run. This makes it easier to iterate quickly on tests without waiting for thousands of unrelated tests to run.
Breaking Changes
Item.Items
Updated to
Items.Group
. You'll see deprecation warnings for usingItem.Items
. Reminder, all deprecated APIs will be removed in v1.Header
This PR overhauls the Header component to work with component augmentation. It also fixed some out standing issues with the Header.
See the docs for examples.
Added
sub
propSUI defines "Sub Headers", a type of Header. A Header can also have a "Subheader" as content (children). Our previous API supported only the "Subheader" child. Now we correctly support both "Sub Header" types and "Subheader" children.
Removed
H*
Sub ComponentsWe previously provided an API like so
<Header.H1 />
. Now you should<Header as='h1' />
.Shorthand props
Shorthand props and
children
are now exclusive, as they should've been from the start. Thecontent
prop has also been added to complete the shorthand prop API.