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

Text to speech block in app lab and game lab #34919

Merged
merged 17 commits into from Jun 4, 2020

Conversation

JillianK
Copy link
Contributor

Added a text to speech block to app lab and game lab that takes in text, output voice gender, and language (currently only english is supported). The block is under UI in app lab and World in game lab. It defaults to female and en-us if given invalid input.
In app lab:
Screen Shot 2020-05-20 at 4 39 39 PM

In game lab:
Screen Shot 2020-05-20 at 4 42 34 PM

as a block:
Screen Shot 2020-05-20 at 4 42 02 PM

Future work

At some point, we need to go back and add support for more languages.

Links

Testing story

There's a test for the arguments of play speech which I think should be sufficient (since we aren't pausing or stopping speech) but let me know if there's something else we need.

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

@JillianK JillianK requested a review from a team as a code owner May 20, 2020 23:52
@JillianK JillianK force-pushed the applab-text-to-speech-block branch from 0840dfe to a7934f0 Compare May 21, 2020 00:18
@JillianK JillianK marked this pull request as draft May 21, 2020 00:20
@JillianK JillianK marked this pull request as ready for review May 21, 2020 16:03
.drone.yml Outdated Show resolved Hide resolved
Sound.prototype.getPlayableBytes = function() {
try {
if (!window.Audio) {
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

why false not undefined or null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was following the style of getPlayableFile (the function above). Should I switch to undefined?

apps/src/Sound.js Outdated Show resolved Hide resolved
@jmkulwik
Copy link
Contributor

IMO: Since we only support English, we shouldn't offer the option to change languages. That feels like a broken promise to me. We should add that option when we add more languages.

apps/src/Sound.js Outdated Show resolved Hide resolved
apps/src/Sound.js Outdated Show resolved Hide resolved
@jmkulwik
Copy link
Contributor

jmkulwik commented May 26, 2020

When I run this locally (Windows 10, Chrome), using the default block playSpeech("Hello World!", "female");, I get this error in the Applab debug console:
image

EDIT: Wait a minute!! I wasn't auto-rebuilding after changes. Let me check that again.

@JillianK
Copy link
Contributor Author

When I run this locally (Windows 10, Chrome), using the default block playSpeech("Hello World!", "female");, I get this error in the Applab debug console:
image

EDIT: Wait a minute. I wasn't auto-rebuilding after changes. Let me check that again.

If you want to run locally, you need keys from me

Copy link
Contributor Author

@JillianK JillianK left a comment

Choose a reason for hiding this comment

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

Just moved the block behind an experiment, text-to-speech-block, and removed it from the toolbox. To access it now, you need to enable the experiment and type in the block

apps/src/lib/util/audioApi.js Outdated Show resolved Hide resolved
apps/src/util/experiments.js Outdated Show resolved Hide resolved
dashboard/app/helpers/levels_helper.rb Outdated Show resolved Hide resolved
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!!

@maddiedierker
Copy link
Contributor

almost forgot -- before you merge this, let's add the API keys to each environment together. we can do this after today's standup

@JillianK JillianK merged commit 0e92eb2 into staging Jun 4, 2020
@JillianK JillianK deleted the applab-text-to-speech-block branch June 4, 2020 19:31
@JillianK JillianK mentioned this pull request Jun 22, 2020
7 tasks
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.

None yet

5 participants