Skip to content

pass non-remarkable props down to the container#30

Closed
chiel wants to merge 1 commit intoacdlite:masterfrom
chiel:master
Closed

pass non-remarkable props down to the container#30
chiel wants to merge 1 commit intoacdlite:masterfrom
chiel:master

Conversation

@chiel
Copy link

@chiel chiel commented Oct 12, 2017

I saw PR #14 but the author is not responding to the requested changes so I decided to open this.

It passes all props that are not used by the Remarkable component itself down to the container. I kept the keyword var though I suppose this could be const.

var Container = this.props.container;
var { container: Container, ...props } = this.props;

['children', 'options', 'source'].forEach(prop => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should container be added here as well?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need, it's not part of props due to the destructuring above.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I'm blind. Carry on 👍
Would love to see this merged soon - was just about to make a PR as well until I saw yours :)

@MrSauceman
Copy link

Would be great to merge this in!

Copy link

@matiasfha matiasfha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. Would be nice to have it in the release

beausmith added a commit to HelloKip/react-remarkable that referenced this pull request Apr 5, 2018
beausmith added a commit to HelloKip/react-remarkable that referenced this pull request Apr 5, 2018
beausmith added a commit to HelloKip/react-remarkable that referenced this pull request Apr 5, 2018
beausmith added a commit to HelloKip/react-remarkable that referenced this pull request Apr 5, 2018
@chiel chiel closed this Apr 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments