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

allow to pass value as props #34

Open
fdecampredon opened this issue Dec 16, 2015 · 8 comments
Open

allow to pass value as props #34

fdecampredon opened this issue Dec 16, 2015 · 8 comments

Comments

@fdecampredon
Copy link

<FormatDate options={{date: 'short'}} >{date}</FormatDate>

can feel a bit awkward could this module also support this api ?

<FormatDate value={date} options={{date: 'short'}} />
@rxaviers
Copy link
Member

Hi,

Currently, whatever is passed in value gets overwritten by children (code below). We could change the below for not overwritten args[0] if children is falsy. so we enable both usages in your description.

// src/generator.js:
this.args[0] = this.props.children;

Would you like to submit a pull request with such change?

@fdecampredon
Copy link
Author

I'll do so !

@kborchers
Copy link
Collaborator

Hi @fdecampredon. Are you still interested in working on this?

@cowboy
Copy link

cowboy commented Mar 11, 2016

I had the same question. I'd love to see this implemented. Basically, use this.props.value if set, otherwise use this.props.children.

@gnarf
Copy link

gnarf commented Mar 11, 2016

I actually think that you should only allow value= prop and deprecate the children format.

@cowboy
Copy link

cowboy commented Mar 11, 2016

I actually think that you should only allow value= prop and deprecate the children format.

👍 to that

@kborchers kborchers mentioned this issue Mar 14, 2016
@rxaviers
Copy link
Member

@gnarf I see you have a preference, but the motivation isn't clear to me. Using a value attribute instead of children might be convenient in the given example (I agree), but using the opposite (children instead of a value attribute) might be convenient in other cases like:

<FormatMessage>You’re receiving notifications because you commented</FormatMessage>

I hold my preference for "use this.props.value if set, otherwise use this.props.children". I don't see any problem with allowing both options this way.

PRs are still very welcome. Thanks

@kborchers
Copy link
Collaborator

I agree with @rxaviers here. I am working on adding some initial tests to the repo (PR almost ready) and then I will add this "use this.props.value if set, otherwise use this.props.children" functionality along with additional tests

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

No branches or pull requests

5 participants