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

Change floatingLabelText prop type from string to node #308

Open
mdboop opened this issue Jan 24, 2019 · 0 comments
Open

Change floatingLabelText prop type from string to node #308

mdboop opened this issue Jan 24, 2019 · 0 comments

Comments

@mdboop
Copy link

mdboop commented Jan 24, 2019

Currently, floatingLabelText is enforced as a string prop-type, passed to FloatingLabel component through the text prop with same prop type, and then rendered inside the label component. This makes it more difficult to use when you're developing an app that uses an i18n library such as react-intl. Most i18n libraries in React provide a component that accepts an id, but I'll use react-intl as my example.

import { FormattedMessage } from 'react-intl';

function Header() => (
  <h1>
    <FormattedMessage id="app.header" />
  </h1>
);

export default Header;

Some third-party libraries you might work with, or even your own code, might necessitate getting the translated string directly, in which case you'd do something like this:

import { withIntl } from 'react-intl';

function Header({ intl }) => (
  <h1>
    {intl.formatMessage({ id: 'app.header' })}
  </h1>
);

export default withIntl(Header);

These two are equivalent (except that FormattedMessage would wrap the text in a span tag), but usually it's preferable to just have the component. It's more readable, your prop types (or interfaces if you're using TS) are simpler, and overall it's a bit less overhead when writing your code.

The floatingLabelText could easily enforce PropTypes.node instead of string and it would work the same, and afford this use-case.

Pros:

  1. Using this component and working with i18n libraries is more ergonomic because you can pass a component with your string id and be done.
  2. Not a breaking change

Cons:

  1. Could be misused by passing some component with conflicting styles, or some complex component that just doesn't belong there (although there's nothing stopping you from doing that now except a prop types warning).
  2. Maybe not significant enough to warrant a change.

I'd love to hear thoughts on this.

@mdboop mdboop changed the title Change floatingLabelText prop type from string to node or add floatingLabel prop Change floatingLabelText prop type from string to node Jan 24, 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

No branches or pull requests

1 participant