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

Fixed grammar #1938

Merged
merged 2 commits into from
Mar 15, 2023
Merged

Fixed grammar #1938

merged 2 commits into from
Mar 15, 2023

Conversation

passchn
Copy link
Contributor

@passchn passchn commented Mar 12, 2023

IBAN is feminin in German: https://www.duden.de/rechtschreibung/IBAN

What I did

  1. Changed spelling/grammar of the german error message for an invalid IBAN.

IBAN is female in German.
@changeset-bot
Copy link

changeset-bot bot commented Mar 12, 2023

🦋 Changeset detected

Latest commit: 47aa452

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@lion/ui Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@CLAassistant
Copy link

CLAassistant commented Mar 12, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@gerjanvangeest gerjanvangeest left a comment

Choose a reason for hiding this comment

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

thanks for the fix 💪

Can you add a changeset (npm run changeset) so it will end up in our changelog?

@tlouisse
Copy link
Member

tlouisse commented Mar 13, 2023

thanks for the fix 💪

Can you add a changeset (npm run changeset) so it will end up in our changelog?

Good that you point this out indeed. However, we have to deal with some edge cases.

Maybe it can be solved with the same approach as in the Dutch language? https://github.com/ing-bank/lion/blob/master/packages/ui/components/input-iban/translations/nl.js#L3
So that would be ein(e) gültige(s) in German?

Why that's needed

Since {fieldName} (corresponding to the .fieldName prop of the FormControl) can be:

  • the .fieldName (when provided by the user)
  • the .label (when no .fieldName provided)

Since we don't know whether the noun (IBAN or anything else) is going to be feminine or not, we don't know what our adjective (gültiges/gültige) + article ("ein" vs "eine") should be.

💡 Possible ways of making this better...

Although ein(e) gültige(s) would provide a valid outcome in all scenarios, it's not the nicest thing to read as an end user.
We might be able to improve UX by looking into a way to also configure the adjective and article of a Validator message for a certain article+adjective+fieldName combo

@passchn
Copy link
Contributor Author

passchn commented Mar 13, 2023

I can't really think of a probable scenario where the noun is neutrum in case of the IBAN field's label.

Useful labels are "IBAN", "Nummer", maybe "Kontonummer" which are all feminin. You really have to be creative to come up with a neutrum word.

Anyway, it would be great to be able to configure the messages, I think! Maybe they could also be passed as attributes to the WebComponent.

@tlouisse
Copy link
Member

I can't really think of a probable scenario where the noun is neutrum in case of the IBAN field's label.

Useful labels are "IBAN", "Nummer", maybe "Kontonummer" which are all feminin. You really have to be creative to come up with a neutrum word.

Anyway, it would be great to be able to configure the messages, I think! Maybe they could also be passed as attributes to the WebComponent.

It's definitely a more sensible default than wat it was before, but in case the label would be "Iban Text", the fieldName would be masculine for instance...

Having a closer look at it: in German it can be gültiger|gültige| gültiges etc.: https://www.verbformen.com/declension/adjectives/gu3ltig.htm

So a one-size-fits-all solution like we have in Dutch is not possible for the German language.

The best solution for now will then be:

  • use the translation you proposed
  • give lion-input-iban a default fieldName of 'iban' (then it takes precedence over the label text and it will always be correct, unless the developer overrides .fieldName)

In case the user overrides .fieldName, it will always be possible to override the complete message.

@tlouisse tlouisse merged commit 183c86a into ing-bank:master Mar 15, 2023
@tlouisse
Copy link
Member

Thanks @passchn!
Added this issue as a follow-up for further finetuning: #1940

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.

4 participants