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

P20-522: Fix Express22 level #54789

Merged
merged 3 commits into from
Nov 15, 2023
Merged

Conversation

artem-vavilov
Copy link
Contributor

@artem-vavilov artem-vavilov commented Nov 8, 2023

Issue

In the level Express22 when translated into French, if you press run (Demarrer) and then click one of the sprites, instead of the sprites moving around, the screen just stays orange. Nothing comes up in the dev console.
1

Solution

Translate the gamelab_ifVarEquals block type NUM variables.
2

Links

Copy link
Contributor

@carl-codeorg carl-codeorg left a comment

Choose a reason for hiding this comment

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

Thanks for going in and fixing this! Works locally for me now, and the proper variable name shows up in the generated code for running the correct function after the user selects a sprite, for example:

ifVarEquals(_C3_A9crire, ('image_' + "cuteanimals_bunny3_1"), function () {

Instead of:

ifVarEquals(type, ('image_' + "cuteanimals_bunny3_1"), function () {

@mgc1194 mgc1194 requested a review from mikeharv November 8, 2023 20:25
Copy link
Contributor

@mgc1194 mgc1194 left a comment

Choose a reason for hiding this comment

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

Translating id may be risky, but since we use the name when calling the function, this change may not have an impact.
We should keep an eye on Honeybadger and Zendesk after these changes get merged.

Copy link
Member

@daynew daynew left a comment

Choose a reason for hiding this comment

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

Looks good, just one question

Comment on lines 611 to 612
# since we call the function by translated name (in `procedures_callnoreturn` blocks),
# we should also translate the `id` attribute that defines the function name
Copy link
Member

Choose a reason for hiding this comment

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

And is calling by the translated name the right thing to do? I would expect id to be an attribute which is never translated. Would it be reasonable to change the calling code to use the untranslated id? I'm guessing not since it is probably generated Blockly code, but I wanted to try asking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, we call the function by its translated name but define the function by the original English name (the id attribute).

So, we either translate the name of the function or call it by its original English name.

I chose the first option because it doesn't impact the level UX.

I also tested this in Hebrew, Chinese and two Arabic languages, and there were no issues with defining functions in these languages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mgc1194 what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Molly took a look and suggested to get Mike's opinion on Monday.

@mgc1194 mgc1194 requested a review from a team November 9, 2023 21:45
@ebeastlake
Copy link
Contributor

Follow-up work tracked here: https://codedotorg.atlassian.net/browse/CT-175

@artem-vavilov artem-vavilov merged commit 6c55565 into staging Nov 15, 2023
2 checks passed
@artem-vavilov artem-vavilov deleted the P20-522/fix-express-22-levels branch November 15, 2023 12:12
molly-moen added a commit that referenced this pull request Nov 15, 2023
* Tutorial explorer: preview mode on hourofcode.com

* Updated test to account for flakiness.

* Added comment

* Re-enabled test

* hoc2023: AI string updates

* Use use_preview variable

* Update schema cache dump after schema changes.

* Require script_id for SingleSectionExperiment and TeacherBasedExperiment (#54843)

* require script_id on SingleSectionExperiment and TeacherBasedExperiment

* fix and improve unit tests

* fix unit tests

* revert stray schema changes

* Update string and font size

* Split lines for explanation text.

* update injection options (#54818)

* Handle pending and error states for rubric ai evaluation (#54860)

* updated rubric for more useful error messages

* updated rubric settings tests for new errors

* undo debug changes to rubrics_controller

* bug fixes

* added messages for pending and running states

* Dance AI: Add start over button (#54899)

* Dance AI: Add start over button

* don't show on explanation page, add tooltips

* Updates to instructions in HoC progression (#54881)

Updates to instructions in HoC progression

* Allow setting workshop organizer permission (#54896)

* Allow setting workshop organizer permission

* Update Assignable Permissions

* added categories and standards csv files (#54933)

* levelbuilder content changes (-robo-commit)

* staging content changes (-robo-commit)

* * removed empty files on dashboard/public/fonts/gotham (#54906)

* P20-522: Fix Express22 level (#54789)

* P20-522: Translate `gamelab_ifVarEquals` block type NUM vsriables

* P20-522: Translate `procedures_defnoreturn` block id attr

* Revert "P20-522: Translate `procedures_defnoreturn` block id attr"

This reverts commit e0f4280.

* [DESign2-51] Add Chips component (#54064)

* Created Chips component
* created stories for Chips
* created tests for Chips
* created documentation for Chips
* removed MultiSelectGroup
* used Chips instead of MultiSelectGroup in the code

* Teachers find course button goes to catalog (#54918)

* Teachers find course button goes to catalog

* update test

* staging content changes (-)

* HoC 2023 - Fix missing string on Educators How-to guide (#54932)

* Add skinny banner to code.org/student/middle-high for AFE scholarships (#54907)

* refactor middle-high.haml and remove stylesheet

* add afe_scholarship_skinny_banner.haml view

* update grade labels and add empty alt text to images

* Dance AI: handle RTL in header, footer, and effect/code toggle (#54922)

* * add focus state for toggle code/effect buttons

* * dance ai - explanation area RTL support fixes

* * dance party - song selector RTL support fix

* * updated explanation button rtl languages positioning
* added comment (review fix)

* Center toggle in LTR/RTL, force LTR header even in RTL languages

* RTL header

* Handle RTL in footer buttons

* Fix text/icon spacing in RTL

* Undo spacing change

* Use div instead of span

---------

Co-authored-by: denyslevada <levada.denys@gmail.com>

* standardize counting of events so all are right (#54927)

* Update Neural Networks lesson text on code.org/ai/how-ai-works and hourofcode.com/ai (#54915)

* Update code.org/hourofcode see all Hour of code tutorials section (#54931)

* add see all activities section

* update images and add responsive styles

---------

Co-authored-by: breville <brendan@code.org>
Co-authored-by: Jessica Kulwik <jessica@code.org>
Co-authored-by: Dave Bailey <davidsbailey@users.noreply.github.com>
Co-authored-by: Molly Moen <molly@code.org>
Co-authored-by: Mark Barnes <mark.barnes@code.org>
Co-authored-by: Sanchit Malhotra <85528507+sanchitmalhotra126@users.noreply.github.com>
Co-authored-by: fisher-alice <107423305+fisher-alice@users.noreply.github.com>
Co-authored-by: Brendan Reville <breville@users.noreply.github.com>
Co-authored-by: Dani LaMarca <dani@code.org>
Co-authored-by: levadadenys <levada.denys@gmail.com>
Co-authored-by: Artem Vavilov <11708250+artem-vavilov@users.noreply.github.com>
Co-authored-by: Kelby Hawn <9256643+kelbyhawn@users.noreply.github.com>
Co-authored-by: bencodeorg <ben@code.org>
Co-authored-by: Turner Riley <56283563+TurnerRiley@users.noreply.github.com>
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

6 participants