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

Adding languages other than English to the text to speech block in app lab and game lab #35077

Merged
merged 15 commits into from Jun 30, 2020

Conversation

JillianK
Copy link
Contributor

@JillianK JillianK commented Jun 1, 2020

This adds support for languages other than English for the text to speech block. The languages that are offered are the intersection of languages that have both male and female voices offered by Azure and are supported by Code.org. The default is still the female English (US) voice.

Here's the full list: 2 dialects of English (US and UK), 2 dialects of Spanish (Latin America and Spain), French, German, Hindi, Italian, Japanese, Portuguese, Russian, and 2 dialects of Chinese (Simplified and Traditional).

Screen Shot 2020-06-01 at 11 58 31 AM

This is currently behind the experiment text-to-speech-block, see here

Future work

In the future, we will add profanity filtering and a character limit for the block.

Links

Testing story

I think the tests in the earlier PR are sufficient but let me know if there's something else we should be testing.

Reviewer Checklist:

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

dashboard/app/helpers/levels_helper.rb Outdated Show resolved Hide resolved
dashboard/app/helpers/levels_helper.rb Outdated Show resolved Hide resolved
Base automatically changed from applab-text-to-speech-block to staging June 4, 2020 19:31
@jmkulwik
Copy link
Contributor

I was curious what the experiment flag was, so I went and looked it up. Here it is in case anyone else is curious: text-to-speech-block

@@ -32,6 +33,19 @@ const dropletConfig = {
0: () => getAssetDropdown('audio')
},
assetTooltip: {0: chooseAsset.bind(null, 'audio')}
},
playSpeech: {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to hide this block behind an experiment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's behind an experiment in applab and gamelab and it needs to be added to a lab's specific dropletConfig to show up.

Copy link
Contributor

@jmkulwik jmkulwik left a comment

Choose a reason for hiding this comment

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

Looks great!

apps/test/unit/lib/util/audioApiTest.js Outdated Show resolved Hide resolved
@@ -32,6 +32,10 @@ def self.table
table.select(:unique_language_s, :locale_s).where("locale_s = '#{locale}'").first[:unique_language_s]
end

cached def self.get_native_name_by_locale(locale)
table.select(:native_name_s, :locale_s).where("locale_s = '#{locale}'").to_a
Copy link
Contributor

Choose a reason for hiding this comment

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

does this method error if we pass in an invalid locale?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, it just returns empty.

Comment on lines 489 to 490
p 'hello world'
p Languages.get_native_name_by_locale('fake language')
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: remove stray logging (but thank you for testing this! 😄 )

Copy link
Contributor

@maddiedierker maddiedierker left a comment

Choose a reason for hiding this comment

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

really great work, jillian! left a last comment about a stray logging statement, but after that's fixed, this is good to merge 🎉

@JillianK JillianK merged commit e6bea24 into staging Jun 30, 2020
@JillianK JillianK deleted the tts-block-languages branch June 30, 2020 13:56
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

3 participants