Skip to content

fluent-react: Filter props with <Localized attrs={{…}}> #141

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

Merged
merged 1 commit into from
Feb 1, 2018

Conversation

stasm
Copy link
Contributor

@stasm stasm commented Feb 1, 2018

Fixes #139.

The component now requires the attrs prop to set any localized attributes as props on the wrapped component. attrs should be an object with attribute names as keys and booleans as values.

<Localized id="type-name" attrs={{placeholder: true}}>
    <input
        type="text"
        placeholder="Type your name"
        onChange={}
        value={name}
    />
</Localized>

By default, if attrs is not passed, no attributes will be set. This is a breaking change compared to the previous behavior: in fluent-react 0.4.1 and before <Localized> would set all attributes found in the
translation.

@stasm
Copy link
Contributor Author

stasm commented Feb 1, 2018

@hkasemir Would you mind reviewing this?

@stasm stasm changed the title fluent-react: Filter props set by translations with <Localized attrs={{…}}> fluent-react: Filter props with <Localized attrs={{…}}> Feb 1, 2018
Copy link
Contributor

@hkasemir hkasemir left a comment

Choose a reason for hiding this comment

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

LGTM!

<Localized id="type-name" attrs={{placeholder: true}}>
<input
type="text"
onChange={…}
Copy link
Contributor

Choose a reason for hiding this comment

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

might include a demo placeholder attribute here

<div existing={true} />
));
});

Copy link
Contributor

Choose a reason for hiding this comment

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

nice test coverage - makes the benefits of explicitly disallowing setting attrs clear.


// The default is to forbid all message attributes. If the attrs prop exists
// on the Localized instance, only set message attributes which have been
// explicitly allowed by the developer.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

The <Localized> component now requires the `attrs` prop to set any localized
attributes as props on the wrapped component. `attrs` should be an object with
attribute names as keys and booleans as values.

```jsx
<Localized id="type-name" attrs={{placeholder: true}}>
    <input
        type="text"
        placeholder="Localizable placeholder"
        value={name}
        onChange={…}
    />
</Localized>
```

By default, if `attrs` is not passed, no attributes will be set. This is
a breaking change compared to the previous behavior: in `fluent-react`
0.4.1 and before `<Localized>` would set _all_ attributes found in the
translation.
@stasm stasm merged commit ed3ac64 into projectfluent:master Feb 1, 2018
@stasm stasm deleted the localized-attrs branch February 1, 2018 20:03
stasm added a commit to stasm/fluent.js that referenced this pull request Feb 1, 2018
  - Allow limited markup in translations. (projectfluent#101)

    Translations in Fluent can now include simple HTML-like markup. Elements
    found in translations will be matched with props passed to `<Localized>`.
    These props must be React elements. Their content will be replaced by the
    localizable content found for the corrensponding markup in the
    translation.

    This is a breaking change from `fluent-react` 0.4.1. See migration notes
    below.

    ```properties
    send-comment = <confirm>Send</confirm> or <cancel>go back</cancel>.
    ```

    ```js
    <Localized
        id="send-comment"
        confirm={
            <button onClick={sendComment}></button>
        }
        cancel={
            <Link to="/"></Link>
        }
    >
        <p>{'<confirm>Send</confirm> or <cancel>go back</cancel>.'}</p>
    </Localized>
    ```

    The rendered result will include the props interpolated into the
    translation:

    ```js
    <p>
        <button onClick={sendComment}>Send</button> or <Link to="/">go back</Link>.
    </p>
    ```

    When naming markup elements it's possible to use any name which is a
    valid prop name. Translations containing markup will be parsed using a
    hidden `<template>` element. It creates a safe inert `DocumentFragment`
    with a hierarchy of text nodes and HTML elements. Any unknown elements
    (e.g. `cancel` in the example above) are parsed as `HTMLUnknownElements`.
    `fluent-react` then tries to match all elements found in the translation
    with props passed to the `<Localized>` component. If a match is found,
    the element passed as a prop is cloned with the translated text content
    taken from the `DocumentFragment` used as `children`.

  - Filter props with <Localized attrs={{…}}>. (projectfluent#139, projectfluent#141)

    The `<Localized>` component now requires the `attrs` prop to set any
    localized attributes as props on the wrapped component. `attrs` should be
    an object with attribute names as keys and booleans as values.

    ```jsx
    <Localized id="type-name" attrs={{placeholder: true}}>
        <input
            type="text"
            placeholder="Localizable placeholder"
            value={name}
            onChange={…}
        />
    </Localized>
    ```

    By default, if `attrs` is not passed, no attributes will be set. This is
    a breaking change compared to the previous behavior: in `fluent-react`
    0.4.1 and before `<Localized>` would set _all_ attributes found in the
    translation.

If you're setting localized attributes as props of elements wrapped in
`<Localized>`, in `fluent-react` 0.6.0 you'll need to also explicitly allow
the props you're interested in using the `attrs` prop. This protects your
components from accidentally gaining props they aren't expecting or from
translations overwriting important props which shouldn't change.

```jsx
// BEFORE (fluent-react 0.4.1)
<Localized id="type-name">
    <input
        type="text"
        placeholder="Localizable placeholder"
        value={name}
        onChange={…}
    />
</Localized>
```

```jsx
// AFTER (fluent-react 0.6.0)
<Localized id="type-name" attrs={{placeholder: true}}>
    <input
        type="text"
        placeholder="Localizable placeholder"
        value={name}
        onChange={…}
    />
</Localized>
```

In `fluent-react` 0.4.1 it was possible to pass React elements as _external
arguments_ to localization via the `$`-prefixed props, just like you'd pass
a number or a date. This was a bad localization practice because it
resulted in the translation being split into multiple strings.

```properties
send-comment-confirm = Send
send-comment-cancel = go back
send-comment = { $confirmButton } or { $cancelLink }.
```

```jsx
// Bad practice. This won't work in fluent-react 0.6.0.
<Localized
    id="send-comment"
    $confirmButton={
        <Localized id="send-comment-confirm">
            <button onClick={sendComment}>{'Send'}</button>
        </Localized>
    }
    $cancelLink={
        <Localized id="send-comment-cancel">
            <Link to="/">{'go back'}</Link>
        </Localized>
    }
>
    <p>{'{ $confirmButton } or { $cancelLink}.'}</p>
</Localized>
```

`fluent-react` 0.6.0 removes support for this feature. It is no longer
possible to pass React elements as `$`-prefixed _arguments_ to translations.
Please migrate your code to use markup in translations and pass React
elements as _props_ to `<Localized>`.

In the example above, change `$confirmButton` to `confirm` and `$cancelLink`
to `cancel`. Note that you don't need to wrap the passed element in another
`<Localized>` anymore. In particular, you don't need to assign a new message
id for it. The text for this element will be taken from the `send-comment`
message which can now include the markup for the button and the link.

```properties
send-comment = <confirm>Send</confirm> or <cancel>go back</cancel>.
```

```jsx
// BEFORE (fluent-react 0.4.1)
<Localized
    id="send-comment"
    $confirmButton={
        <Localized id="send-comment-button">
            <button onClick={sendComment}>{'Send'}</button>
        </Localized>
    }
    $cancelLink={
        <Localized id="send-comment-cancel">
            <Link to="/">{'go back'}</Link>
        </Localized>
    }
>
    <p>{'{ $confirmButton } or { $cancelLink}.'}</p>
</Localized>
```

```jsx
// AFTER (fluent-react 0.6.0)
<Localized
    id="send-comment"
    confirm={
        <button onClick={sendComment}></button>
    }
    cancel={
        <Link to="/"></Link>
    }
>
    <p>{'<confirm>Send</confirm> or <cancel>go back</cancel>.'}</p>
</Localized>
```

`fluent-react` 0.6.0 works best with `fluent` 0.6.0. It might still work with
`fluent` 0.4.x but passing elements as `$`-prefixed arguments to translations
will break your app. You might also run into other issues with translations
with attributes and no values. Upgrading your code to [`fluent` 0.6.0][] and
your localization files to [Fluent Syntax 0.5][] is the best way to avoid
troubles.

[`fluent` 0.6.0]: https://github.com/projectfluent/fluent.js/releases/tag/fluent%400.6.0
[Fluent Syntax 0.5]: https://github.com/projectfluent/fluent/releases/tag/v0.5.0
stasm added a commit to stasm/fluent.js that referenced this pull request Feb 1, 2018
  - Allow limited markup in translations. (projectfluent#101)

    Translations in Fluent can now include simple HTML-like markup. Elements
    found in translations will be matched with props passed to `<Localized>`.
    These props must be React elements. Their content will be replaced by the
    localizable content found for the corrensponding markup in the
    translation.

    This is a breaking change from `fluent-react` 0.4.1. See migration notes
    below.

    ```properties
    send-comment = <confirm>Send</confirm> or <cancel>go back</cancel>.
    ```

    ```js
    <Localized
        id="send-comment"
        confirm={
            <button onClick={sendComment}></button>
        }
        cancel={
            <Link to="/"></Link>
        }
    >
        <p>{'<confirm>Send</confirm> or <cancel>go back</cancel>.'}</p>
    </Localized>
    ```

    The rendered result will include the props interpolated into the
    translation:

    ```js
    <p>
        <button onClick={sendComment}>Send</button> or <Link to="/">go back</Link>.
    </p>
    ```

    When naming markup elements it's possible to use any name which is a
    valid prop name. Translations containing markup will be parsed using a
    hidden `<template>` element. It creates a safe inert `DocumentFragment`
    with a hierarchy of text nodes and HTML elements. Any unknown elements
    (e.g. `cancel` in the example above) are parsed as `HTMLUnknownElements`.
    `fluent-react` then tries to match all elements found in the translation
    with props passed to the `<Localized>` component. If a match is found,
    the element passed as a prop is cloned with the translated text content
    taken from the `DocumentFragment` used as `children`.

  - Filter props with <Localized attrs={{…}}>. (projectfluent#139, projectfluent#141)

    The `<Localized>` component now requires the `attrs` prop to set any
    localized attributes as props on the wrapped component. `attrs` should be
    an object with attribute names as keys and booleans as values.

    ```jsx
    <Localized id="type-name" attrs={{placeholder: true}}>
        <input
            type="text"
            placeholder="Localizable placeholder"
            value={name}
            onChange={…}
        />
    </Localized>
    ```

    By default, if `attrs` is not passed, no attributes will be set. This is
    a breaking change compared to the previous behavior: in `fluent-react`
    0.4.1 and before `<Localized>` would set _all_ attributes found in the
    translation.

If you're setting localized attributes as props of elements wrapped in
`<Localized>`, in `fluent-react` 0.6.0 you'll need to also explicitly allow
the props you're interested in using the `attrs` prop. This protects your
components from accidentally gaining props they aren't expecting or from
translations overwriting important props which shouldn't change.

```jsx
// BEFORE (fluent-react 0.4.1)
<Localized id="type-name">
    <input
        type="text"
        placeholder="Localizable placeholder"
        value={name}
        onChange={…}
    />
</Localized>
```

```jsx
// AFTER (fluent-react 0.6.0)
<Localized id="type-name" attrs={{placeholder: true}}>
    <input
        type="text"
        placeholder="Localizable placeholder"
        value={name}
        onChange={…}
    />
</Localized>
```

In `fluent-react` 0.4.1 it was possible to pass React elements as _external
arguments_ to localization via the `$`-prefixed props, just like you'd pass
a number or a date. This was a bad localization practice because it
resulted in the translation being split into multiple strings.

```properties
send-comment-confirm = Send
send-comment-cancel = go back
send-comment = { $confirmButton } or { $cancelLink }.
```

```jsx
// Bad practice. This won't work in fluent-react 0.6.0.
<Localized
    id="send-comment"
    $confirmButton={
        <Localized id="send-comment-confirm">
            <button onClick={sendComment}>{'Send'}</button>
        </Localized>
    }
    $cancelLink={
        <Localized id="send-comment-cancel">
            <Link to="/">{'go back'}</Link>
        </Localized>
    }
>
    <p>{'{ $confirmButton } or { $cancelLink}.'}</p>
</Localized>
```

`fluent-react` 0.6.0 removes support for this feature. It is no longer
possible to pass React elements as `$`-prefixed _arguments_ to translations.
Please migrate your code to use markup in translations and pass React
elements as _props_ to `<Localized>`.

In the example above, change `$confirmButton` to `confirm` and `$cancelLink`
to `cancel`. Note that you don't need to wrap the passed element in another
`<Localized>` anymore. In particular, you don't need to assign a new message
id for it. The text for this element will be taken from the `send-comment`
message which can now include the markup for the button and the link.

```properties
send-comment = <confirm>Send</confirm> or <cancel>go back</cancel>.
```

```jsx
// BEFORE (fluent-react 0.4.1)
<Localized
    id="send-comment"
    $confirmButton={
        <Localized id="send-comment-button">
            <button onClick={sendComment}>{'Send'}</button>
        </Localized>
    }
    $cancelLink={
        <Localized id="send-comment-cancel">
            <Link to="/">{'go back'}</Link>
        </Localized>
    }
>
    <p>{'{ $confirmButton } or { $cancelLink}.'}</p>
</Localized>
```

```jsx
// AFTER (fluent-react 0.6.0)
<Localized
    id="send-comment"
    confirm={
        <button onClick={sendComment}></button>
    }
    cancel={
        <Link to="/"></Link>
    }
>
    <p>{'<confirm>Send</confirm> or <cancel>go back</cancel>.'}</p>
</Localized>
```

`fluent-react` 0.6.0 works best with `fluent` 0.6.0. It might still work with
`fluent` 0.4.x but passing elements as `$`-prefixed arguments to translations
will break your app. You might also run into other issues with translations
with attributes and no values. Upgrading your code to [`fluent` 0.6.0][] and
your localization files to [Fluent Syntax 0.5][] is the best way to avoid
troubles.

[`fluent` 0.6.0]: https://github.com/projectfluent/fluent.js/releases/tag/fluent%400.6.0
[Fluent Syntax 0.5]: https://github.com/projectfluent/fluent/releases/tag/v0.5.0
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