Skip to content
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

Merged
merged 12 commits into from
Aug 25, 2016

Conversation

levithomason
Copy link
Member

@levithomason levithomason commented Aug 21, 2016

Fixes #403. This PR will add component augmentation.

Testing 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 using Item.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 prop

SUI 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 Components

We 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. The content prop has also been added to complete the shorthand prop API.

@codecov-io
Copy link

codecov-io commented Aug 21, 2016

Current coverage is 96.17% (diff: 95.54%)

Merging #414 into master will increase coverage by 1.02%

@@             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          

Powered by Codecov. Last update ca7188e...511860f

@levithomason levithomason force-pushed the feature/component-augmentation branch 3 times, most recently from 5422924 to e7ec341 Compare August 23, 2016 00:24
@levithomason levithomason force-pushed the feature/component-augmentation branch 2 times, most recently from d2598e0 to 007b623 Compare August 25, 2016 16:32
@levithomason levithomason changed the title Add component augmentation Breaking Changes: Add component augmentation Aug 25, 2016
// ----------------------------------------
// user defined element type

if (props.as && props.as !== defaultProps.as) return props.as
Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member Author

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:

  1. User defined an as prop
  2. getElementType() typeMap arg mapped a prop to an element type: { onClick: 'a' }
  3. Inferred type: `href => a'
  4. defaultProp.as
  5. 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',
}

@dvdzkwsk
Copy link
Member

👻

@levithomason levithomason merged commit c274715 into master Aug 25, 2016
@levithomason levithomason deleted the feature/component-augmentation branch August 25, 2016 17:28
@levithomason
Copy link
Member Author

Released in v0.37.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RFC: Consider component augmentation
3 participants