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

Add styling to FormattedRelative #255

Closed
wants to merge 1 commit into from
Closed

Add styling to FormattedRelative #255

wants to merge 1 commit into from

Conversation

nodkz
Copy link
Contributor

@nodkz nodkz commented Dec 22, 2015

Add ability to pass styles and classname to FormattedRelative via props.

Add ability to pass styles and classname to FormattedRelative via props.
@ericf
Copy link
Collaborator

ericf commented Dec 22, 2015

@nodkz I wish you would have looked through the past issues related to style and className props. It's a very explicit design decision to now allow these so that the API of the <Formatted*> components can remain tight and rendering is deterministic.

You should think of the <Formatted*> components like text and wrap another primitive element, e.g., <span> around them to add styling. All the components also support the function-as-child pattern so a function can be passed as the child and the component will delegate to it for rendering.

There's a great talk from Sebastian Markbage on this topic.

If you have some new ideas to add on this topic after reviewing the other related issues, please feel free to comment on those.

@nodkz
Copy link
Contributor Author

nodkz commented Dec 22, 2015

Ok I'll see talk from Sebastian.

I just want to avoid wrapping with another primitive element. For my opinion it looks ugly (but I'll may change my opinion after Sebastian's talk):

<div className="statistics">
  { data.stats && data.stats.views &&
    <div className="statistics-views">
      <FormattedMessage
        defaultMessage={`{ num, plural, =0 {} one {# человек читал его.} few {# человека читали его.} many {# человек читали его.} other {} }`}
        values={{ num: data.stats.views }} />
    </div>
  }
  { data.stats && data.stats.purchases &&
    <div className="statistics-purchases">
      <FormattedMessage
        defaultMessage={`{ num, plural, =0 {} one {# купил доступ к контактам.} few {# купили доступ к контактам.} many {# купили доступ к контактам.} other {} }`}
        values={{ num: data.stats.purchases }} />
    </div>
  }
</div>

VERSUS

<div className="statistics">
  { data.stats && data.stats.views &&
      <FormattedMessage
        className="statistics-views"
        defaultMessage={`{ num, plural, =0 {} one {# человек читал его.} few {# человека читали его.} many {# человек читали его.} other {} }`}
        values={{ num: data.stats.views }} />
  }
  { data.stats && data.stats.purchases &&
      <FormattedMessage
        className="statistics-purchases"
        defaultMessage={`{ num, plural, =0 {} one {# купил доступ к контактам.} few {# купили доступ к контактам.} many {# купили доступ к контактам.} other {} }`}
        values={{ num: data.stats.purchases }} />
  }
</div>

@nodkz
Copy link
Contributor Author

nodkz commented Dec 23, 2015

Yep, Sebastian as always speaks great things! Thanks for provided link, it helps me understood further React movement.

I agree that passing styles and classNames is solution only for DOM-model, but not for all react-world (native, canvas, curses).

Thanks again. And thank you for such great intl implementation for reat. v2.0.0 is awesome!

@ericf
Copy link
Collaborator

ericf commented Dec 23, 2015

Cool! Thanks for offering up these contributions, even though they weren't merged in the end. I hope you contribute again in the future. If there's something that's not an obvious bug, feel free to open an issue first and we can discuss it before you spend too much time implementing a change 😄

longlho added a commit that referenced this pull request Apr 27, 2020
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.

2 participants