TTS for Contained Levels #26381
TTS for Contained Levels #26381
Conversation
| Given I am a student | ||
| And I am on "http://studio.code.org/s/allthettsthings/stage/1/puzzle/1" | ||
| And I wait for the page to fully load | ||
|
|
||
| # note: we expect audio for csd instructions |
There was a problem hiding this comment.
Nit: This comment refers to CSD but this test now covers CSF (I think?)
| Then I wait until element ".inline-audio" is visible | ||
| Then I see 1 of jquery selector .inline-audio | ||
| #Checks that inline audio does not disappear (indication of error) | ||
| And I listen to the 0th inline audio element |
There was a problem hiding this comment.
😂 We should really extend this step so we can write "first" instead of "0th."
There was a problem hiding this comment.
I would worry about confusion between the "first" and "1st" cases :/
| all_instructions = [] | ||
|
|
||
| contained_levels.each {|contained| all_instructions.push(contained_level_text(contained))} | ||
| contained_levels.empty? ? nil : all_instructions * "\n" |
There was a problem hiding this comment.
should this be all_instructions.empty??
There was a problem hiding this comment.
My original intention here was: 'if there are no contained levels, then just return nil', but implementation-wise I agree that checking all_instructions makes more sense.
|
@islemaster or @Hamms - Can I get one more look at the short_instructions part? Thanks! |
| @@ -197,7 +198,7 @@ def tts_for_contained_level | |||
| def contained_level_text(contained) | |||
| # For multi questions, create a string for TTS of the markdown, question, and answers | |||
| if contained.long_instructions.nil? | |||
| combined_text = contained.properties["markdown"] + "\n" | |||
| combined_text = contained.properties["markdown"].nil? ? "" : contained.properties["markdown"] + "\n" | |||
There was a problem hiding this comment.
why is the "\n" necessary here?
There was a problem hiding this comment.
Mostly, a space is needed to make sure that when the strings are concatenated the last word of one string and the first word of the next don't become one word - which Sharon would then read as one word. I just chose a "\n" for debugging simplicity, but we could use any other whitespace there.
|
|
||
| test 'tts_short_instructions_text for contained levels' do | ||
| contained_level_freeresponse = create :level, name: 'contained level 1', type: 'FreeResponse', properties: { | ||
| markdown_instructions: "This is contained" |
There was a problem hiding this comment.
nit: we prefer long_instructions over markdown_instructions for this now
There was a problem hiding this comment.
We don't display short instructions for contained levels, but we want to support TTS on contained levels. When I turned on TTS on those levels, the levels which default to short instructions were playing the audio for instructions that are not (I think, never) displayed. So this change is designed to override those situations - if there are contained levels, we'll use that TTS audio, whether or not the containing level defaults to short or long instructions (since those short/long instructions shouldn't be displayed, as far as I know).
Remove short instrucitons adding contained level text
Specifically, update the script which generates non-English TTS audio to use the general "tts_*_text" methods, which enables them to see the new contained levels content added in #26381
Adds TTS for Contained Levels on CSF and CSD/P
For free response levels: the prompt is read
For multiple choice levels: the context, question, and answers are read
Because contained levels in CSF are not displayed in the white speech-bubble, the CSS for the inline audio has been slightly updated, per design spec from Mark.
CSF TTS on non-contained level:
CSF TTS on contained level:
The new audio files have not been generated yet and should be generated prior to this PR getting merged.