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

feat(curriculum): add animations to block 4.1 of the English Curriculum #54348

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

DanielRosa74
Copy link
Contributor

Checklist:

@DanielRosa74 DanielRosa74 requested a review from a team as a code owner April 11, 2024 08:00
@github-actions github-actions bot added the scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. label Apr 11, 2024
@moT01 moT01 added the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label Apr 12, 2024
Copy link
Member

@moT01 moT01 left a comment

Choose a reason for hiding this comment

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

This is looking real good @DanielRosa74 🎉


Quite a few of the animations seem to cut off a little early. Here’s a mostly complete list of the ones I noticed: Tasks 9, 20, 21, 25, 34, 38, 46, 47, 51-67, 70-72, 74, 77, 80, 87, 88, 99, 109, 110, 115, 116, 119, 120, 121, 127, 128, 137-139. Some of them are minor enough that they could probably just be left alone, but others are quite significant. Are you seeing that? Perhaps there's an issue with how something is loaded and you aren't seeing it. Or maybe it's an issue with the script that creates the scenes, or how it's played - not sure.


Tasks 66 and 67 have the same JSON for the scene, but they seem to cut off at slightly different times.








Tasks 9 and 10 end with Maria saying the same word, but the finishTimestamp is different. Not sure if you manually changed that or if the script is being goofy.












Before we go in and change all the times, I think we should look into a little and see where the issue is. Seems like it's probably something with the finishTime, since the majority of the scenes work fine, but it would be nice to be sure.
















Task 134-136 open-source doesn't have the hyphen.

Copy link
Contributor

@nieldakarla nieldakarla left a comment

Choose a reason for hiding this comment

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

@DanielRosa74 I'll add my notes here. @moT01, I've paid special attention to the tasks you mentioned on your review. I've listed below the tasks I've noticed the audio gets cut off earlier and the ones that have extra words.

Tasks with extra words from other lines of dialogue:
12, 42, 83, 84, 101, 102, 124, 131 and 133

Tasks that get cut off before the end:
52, 99, 119 (there might be some more that could use some breathing space, but I didn't mention them here)

About tasks 9 and 10:

I think they are correct. From what I saw, the difference is that task 9 uses just the second part of what Maria is saying - The first part is in task 08 - while task 10 uses her entire line of dialogue to check understanding.

Extra requests:
Task 44: Can you please update the question to be: "What is making Tom feel demotivated?"
Currently it ends with "and why?" but the options don't include reasons.

Task 126: Remove extra dot at the end of the description.

@naomi-lgbt naomi-lgbt added status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer. and removed status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. labels Apr 25, 2024
@moT01 moT01 added the new english course English Curriculum label May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new english course English Curriculum scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants