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

FreeResponse levels have translated instructions. #32650

Merged
merged 1 commit into from
Jan 15, 2020

Conversation

daynew
Copy link
Member

@daynew daynew commented Jan 13, 2020

Description

Issue: FreeResponse levels were not showing translated
instructions even though translations existed.
Cause: FreeResponse levels not using the I18n API for content.
Fix: Update the long_instructions to be grabbed using the I18n API.

  • Added new free_response.placeholder string so we can translate the default placeholder text "Enter your answer here"

Links

Follow-up work

Screenshots

Before

Note that the "Make a prediction..." text is not translated to Japanese.
image

After

image

Testing story

Reviewer Checklist:

  • Tests provide adequate coverage
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@daynew daynew force-pushed the fnd-772-free-response-level-i18n branch from 327edeb to f4614bc Compare January 13, 2020 23:03
@daynew daynew requested review from bethanyaconnor and a team January 13, 2020 23:59
@daynew daynew marked this pull request as ready for review January 14, 2020 00:03
* Issue: FreeResponse levels were not showing translated
instructions even though translations existed.
* Cause: FreeResponse levels not using the I18n API for content.
* Fix: Update the `long_instructions` to be grabbed using the I18n API.

* Also added new string: free_response.placeholder which will allow
translators to translate the text "Enter your answere here".
@daynew daynew force-pushed the fnd-772-free-response-level-i18n branch from f4614bc to abd3e49 Compare January 14, 2020 21:42
@daynew
Copy link
Member Author

daynew commented Jan 15, 2020

I have fixed the UI tests:

  • The UI tests were failing because I was incorrectly calling the I18n API for en-US locale. I have updated the code to use the database value for en-US locale and the I18n API for non-en-US. I have also set the database value as the default if the I18n returns nil because there is no translation yet.

def get_property(property_name)
# Return the default English string from the database model if we shouldn't
# localize this property or we couldn't find a localized value.
default = try(property_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah we moved the source of truth for some levels to the DB because we didn't want to upload everything to Crowdin iirc.

Hopefully as part of a shared API we can define where the English source strings for all levels...

@bethanyaconnor
Copy link
Contributor

Pulled and tested locally and it looks good! I don't love the fact that the source of truth is different for different levels but definitely a followup (that I believe you already filed).

@daynew daynew merged commit ff68933 into staging Jan 15, 2020
@daynew daynew deleted the fnd-772-free-response-level-i18n branch January 15, 2020 23:25
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.

None yet

2 participants