Skip to content

Check in French translations#1523

Merged
zachmargolis merged 1 commit intomasterfrom
margolis-add-translations
Jul 5, 2017
Merged

Check in French translations#1523
zachmargolis merged 1 commit intomasterfrom
margolis-add-translations

Conversation

@zachmargolis
Copy link
Contributor

Why: Results back from vendor

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this file needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it is. It looks like it didn't get properly populated. @zachmargolis Is this the parsing issue you were mentioning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'll have to take a look at this, oops

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I pushed up the translations and fixed up the commit

Copy link
Contributor

@nickbristow nickbristow left a comment

Choose a reason for hiding this comment

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

show password checkbox not translated

Looks like may be an issue with my setup according to @monfresh

@monfresh
Copy link
Contributor

monfresh commented Jul 3, 2017

@nickbristow On which screen are you not seeing the "Show password" checkbox in French? I see that the forms.passwords.show entry in locales/forms/fr.yml is translated. I also see it on the home page when running the app locally. See screenshot below:

screen shot 2017-07-03 at 12 42 13 pm

@nickbristow
Copy link
Contributor

@monfresh it might just be the manage or change password page.

login_gov_-_modifier_votre_mot_de_passe

@nickbristow
Copy link
Contributor

nickbristow commented Jul 3, 2017

@monfresh im not sure whats happening
login_gov_-_bienvenue

@zachmargolis zachmargolis force-pushed the margolis-add-translations branch from 8064fee to d0ed55d Compare July 3, 2017 18:23
**Why**: Results back from vendor
@monfresh
Copy link
Contributor

monfresh commented Jul 3, 2017

I think it's easier to test this by running the app locally and looking at all the screens. Reading through the code is not a good way to review this PR, but it looks OK at first glance. I vote to merge this in, then test it thoroughly on dev and open any bugs we find.

@monfresh
Copy link
Contributor

monfresh commented Jul 3, 2017

I'm seeing the same bug as @nickbristow now. Let's open an issue for it.
I'm also not seeing the password strength feedback being translated. It's probably a JS issue.

Copy link
Contributor

@monfresh monfresh left a comment

Choose a reason for hiding this comment

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

LGTM

@zachmargolis zachmargolis merged commit c97acca into master Jul 5, 2017
@zachmargolis zachmargolis deleted the margolis-add-translations branch July 5, 2017 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants