search

fiverr / passable

Declarative data validations.
https://fiverr.github.io/passable/
MIT License
100 stars 11 forks source link

[FEAT] Support for React Elements #394

Closed ghost closed 2 years ago

ghost commented 2 years ago

⚠️ Please Ignore whitespaces when reviewing (Had some Lint indentation auto fixes).

Q A
Bug fix? no
New feature? yes
Breaking change? no
Deprecations? no
Documentation? yes
Tests added? yes

What

This PR uses the RC release of passable (Link to PR is below in References) where it support using the new localization template <t name="i">Italic</t> inside the validation messages whether it's warnings or errors. (span, b, br, i, etc)

Up until today, passable was supporting using HTML tags as we did before in the copies.json. Today, the localization team uses a new feature called predefined templates <t> and it's the only tag that can be used inside the copies.

One of the conditions of using the template feature is to render it inside the React tree as a React element for it to work.

Until today, Passable only worked with errors and warnings defined as strings only and with the new Infra changes related to the copies we can't use the templates feature because passable ignores any non-string values (whether by using <I18n /> or translate('...')).

Since passable only supported strings we were enforced to deal with two way of writing by using the I18n component in the UI components and using translate() inside the passable validations.

With this new approach, we can use only one format <I18n/> with also the support for the defined and the customized templates (e.g. <t name="icn">Hi</t>)!

Screenshots (From Gig Edit Page)

Here we're rendering one of the messages by using the I18n React component 👇

image

When we migrated 2 days ago to react-i18n we were enforced to remove the Italic but with this proposal we can get it back again 👇

image
ealush commented 2 years ago

I would advise against making such a change. Making an API change of this kind means moving farther away from the premise of "unit-test-like" interface. Instead, I would suggest providing a way to expose information from within the test.

ghost commented 2 years ago

I would advise against making such a change. Making an API change of this kind means moving farther away from the premise of "unit-test-like" interface. Instead, I would suggest providing a way to expose information from within the test.

PMing you to explain why this is needed.