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

Simple translate strings #7

Closed
tomasz89nowak opened this issue Jan 15, 2018 · 11 comments
Closed

Simple translate strings #7

tomasz89nowak opened this issue Jan 15, 2018 · 11 comments

Comments

@tomasz89nowak
Copy link

tomasz89nowak commented Jan 15, 2018

Hi, Your library has almost all I need for my project and is simple and clear to use, but I can't figure out how to translate input placeholders, html element title or just string in constructor. Is it even possible? If not, do You plan to add HOC or render prop component to use it like:

<input placeholder={t('something')} />
constructor(props) {    
  super(props); 
  this.defaultText = props.t('something');
}

or

<Translate text="something">
  {(translatedText) => (
    <input placeholder={translatedText} />
  )}
</Translate>

EDIT

🙌 Possible solution

I wrote simple HOC to handle my problem, it need some improvements but works for a simple case

function withTranslated(Component) {
  return class TranslatorWrapper extends React.Component {
    static contextTypes = {
      translated: PropTypes.shape({
        getLanguage: PropTypes.func.isRequired,
        getTranslation: PropTypes.func.isRequired,
        subscribe: PropTypes.func.isRequired,
      }).isRequired,
    }

    constructor(props, context) {
      super(props, context);
      this.translation = this.context.translated.getTranslation();
      this.lang = this.context.translated.getLanguage();
    }

    getTranslation = (tag) => {
      if(this.translation[tag] && this.translation[tag][this.lang]) {
        return this.translation[tag][this.lang];
      } else {
        console.error(`lack of translation "${tag}"`);
        return tag;
      }
    }

    render() {
      return <Component {...this.props} t={this.getTranslation} />;
    }
  }
}
@amsul
Copy link
Owner

amsul commented Jan 15, 2018

@tomasz89nowak that's a great suggestion! I like your idea for it as a render prop. I'll push an update soon that has that feature :)

amsul added a commit that referenced this issue Jan 15, 2018
@amsul amsul mentioned this issue Jan 15, 2018
@amsul amsul closed this as completed in #8 Jan 15, 2018
@amsul
Copy link
Owner

amsul commented Jan 17, 2018

@tomasz89nowak this is now available in v2.1.0 :)

@tomasz89nowak
Copy link
Author

@amsul thanks, I saw it, great job! I will use it, but please consider making HOC too or provide translating function as a render prop, because translating form with 15 inputs with translated placeholders and titles is pretty complex now :).

<Translate
  text='placeholder'
  render={({ translatedText: translatedPlaceholder }) => (
    <Translate
      text='title'
      render={({ translatedText: translatedTitle }) => (
        <input placeholder={translatedPlaceholder} title={translatedTitle} />
      )}
    />
  )}
  />

Proposition:

<Translate
  render={({ translate }) => (
    <input placeholder={translate('placeholder')} title={translate('title')} />
  )}
/>

What do You think?

@amsul
Copy link
Owner

amsul commented Jan 17, 2018

@tomasz89nowak true! I hadn't thought about it in that much depth.

What do you think about adding a new component (and removing the custom renderer I added):

<Translator>
  {({ translate }) => (
    <input
      placeholder={translate({ text: 'placeholder' })}
      title={translate({
        text: 'Hi {firstName}',
        data: { firstName: 'Sergey' },
      })}
    />
  )}
</Translator>

@tomasz89nowak
Copy link
Author

Looks very good for me, that's all I need :)
I personally prefer syntax as below, because 99% of my translated texts are just strings, without tags and data. But both options are good and You are the boss here :)

<Translator>
  {({ translate }) => (
    <input
      placeholder={translate('placeholder')}
      title={translate('Hi {firstName}',  {
        data: { firstName: 'Sergey' }
      })}
    />
  )}
</Translator>

@amsul
Copy link
Owner

amsul commented Jan 17, 2018

@tomasz89nowak I just pushed it to master. Can you please try it and see it works for you? I'll push it to npm later.

Regarding the syntax, I agree less characters are usually nice. But for several reasons, I like having named arguments instead of ordered arguments - and the closest we can get to that in JavaScript is with this syntax of "pseudo-named" arguments.

@tomasz89nowak
Copy link
Author

I tried and it works, but it's not always consistent with Translate component:

  • when I pass string not included in translations:
<Translate text="something" /> // returns "something"
<Translator>
  {({ translate }) => (
    <div>{translate({ text: "something" })}</div> // returns ""
  )}
</Translator>
  • when I pass text with tag "something {someData}" without passing data:
<Translate text="something {someData}" /> // it crashes, what is expected behavior for me
<Translator>
  {({ translate }) => (
    <div>{translate({ text: "something {someData}" })}</div> // returns ""
  )}
</Translator>

That's all I found for now.
Also it would be nice to have an option to warn in console use of string not included in translations :)

@amsul
Copy link
Owner

amsul commented Jan 18, 2018

@tomasz89nowak debugging is visibly implemented in the Translate component.

If you create your provider like this: <Provider isDebugging={true}>, then it will make the background of missing strings red. This needs to be documented.

Regarding the empty string for Translator, that's because there's no way to make the string's background red. The rendering is left up to the developer so the onus is on the developer to handle those scenarios.

Although, I agree it would be nice to have a way to indicate it visibly. Logging it to the console is not ideal imo.. Would love to hear some thoughts.

@amsul
Copy link
Owner

amsul commented Jan 18, 2018

Now that I think about it, one option is that Translator could wrap it's entire children and give that node a background color.

Only issue there is that since you can render many children in there, it's difficult to know which one in specific has the missing translation.

@tomasz89nowak
Copy link
Author

I see another issue in wrapping entire children - wrapper must be HTML tag so it will modify the DOM tree, so it can spoil styles like .something > div {...}. And we will need new property for Translator like

<Translator tag="div">

Logging to the console is not ideal, but better than wrapping children in my opinion.

@amsul
Copy link
Owner

amsul commented Feb 9, 2018

True - that's not ideal.

For starters, I guess it should at least default to the string passed in to be translated.

Regarding debugging it, another possible solution is to use Portals and display any missing strings in some kind of overlaid sidebar.

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

2 participants