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

support for optional tagName for components -> FormattedDate, Formatt… #131

Merged
merged 2 commits into from
Jun 30, 2015

Conversation

rsamec
Copy link
Contributor

@rsamec rsamec commented Jun 24, 2015

support for optional tagName for components instead of hardcoded span element.

It solves the issue

@@ -24,12 +24,13 @@ var FormattedDate = React.createClass({

render: function () {
var props = this.props;
var tagName = props.tagName || 'span';

Choose a reason for hiding this comment

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

Specify 'span' as a default value.

ericf added a commit that referenced this pull request Jun 30, 2015
support for optional tagName for components -> FormattedDate, Formatt…
@ericf ericf merged commit af66a33 into formatjs:master Jun 30, 2015
@ericf
Copy link
Collaborator

ericf commented Jun 30, 2015

Just leaving a note here that tagName was added to really only help edge cases like SVG, and if additional styling is desired, a developer should wrap the React Intl components.

@gpbl
Copy link

gpbl commented Jul 1, 2015

@ericf maybe a warning when using tagName with values other than tspan or span?

@elCarda
Copy link

elCarda commented Jul 23, 2015

@ericf Please release it as a minor version? Or any release in roadmap soon?

@ericf
Copy link
Collaborator

ericf commented Aug 20, 2015

Trying to determine if component is better than tagName and will help us support React Native as well…

Also, in the v2-dev branch all of the components now support the function-as-child pattern which helps to mitigate this.

@ericf ericf mentioned this pull request Aug 20, 2015
@SimenB
Copy link

SimenB commented Jan 13, 2016

Chiming in months later with a request to change it to component as noted above. That more closely mirrors React's own ReactCSSTransitionGroup

https://facebook.github.io/react/docs/animation.html#rendering-a-different-component

For all I know 2.0.0 API is frozen though 😄

@ericf
Copy link
Collaborator

ericf commented Jan 13, 2016

@SimenB What's your use case? I won't be adding support for passing various props to the underlying component that's rendered; if you need to customize rendering in that way you can pass a function as the child.

@SimenB
Copy link

SimenB commented Jan 13, 2016

No use case (we already wrap the intl-components), this suggestion was just to rename your prop tagName to component like you suggested above, for the sake of consistency with React itself

@ericf
Copy link
Collaborator

ericf commented Jan 13, 2016

What do you use tagName/component for? To you ever assign a value to it?

@SimenB
Copy link

SimenB commented Jan 13, 2016

We don't use it (or, we did, until I watched "Dom as second-class citizen"). My comment was just on the fact that you named your prop something other than React did, which makes it a bit less intuitive (even though I know its use is discouraged).

@SimenB
Copy link

SimenB commented Jan 13, 2016

Of course, transition group is a container and requires children which intl does not, so it's not the exact same thing

@ericf
Copy link
Collaborator

ericf commented Jan 13, 2016

The driving force for me to support this is rendering in React Native and SVGs which don't have <span>.

@jmz7v jmz7v mentioned this pull request Nov 10, 2016
longlho pushed a commit that referenced this pull request Apr 27, 2020
Add missing sourcemap files
longlho pushed a commit that referenced this pull request Apr 27, 2020
…ate skeleton (#131)

* [intl-messageformat-parser] add parser for number skeleton and date skeleton

* remove unused command
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.

None yet

6 participants