react-globalize icon indicating copy to clipboard operation
react-globalize copied to clipboard

allow to pass value as props

Open fdecampredon opened this issue 10 years ago • 8 comments

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

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

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

fdecampredon avatar Dec 16 '15 09:12 fdecampredon

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?

rxaviers avatar Dec 16 '15 11:12 rxaviers

I'll do so !

fdecampredon avatar Dec 16 '15 13:12 fdecampredon

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

kborchers avatar Mar 11 '16 15:03 kborchers

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

cowboy avatar Mar 11 '16 15:03 cowboy

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

gnarf avatar Mar 11 '16 19:03 gnarf

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

:+1: to that

cowboy avatar Mar 11 '16 19:03 cowboy

@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

rxaviers avatar Mar 15 '16 12:03 rxaviers

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

kborchers avatar Mar 15 '16 13:03 kborchers