-
Notifications
You must be signed in to change notification settings - Fork 350
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
Make the Expression widget treat sen
as equivalent to sin
#921
Conversation
`sen` is used instead of `sin` in Portuguese. This change allows the Expression widget to accept both spellings. We simply convert any instances of `sen` to `sin` when evaluating the learner's answer. Issue: https://khanacademy.atlassian.net/browse/LC-290 Test plan: - Start storybook: `yarn storybook`. - Visit `http://localhost:6006/?path=/docs/perseus-widgets-expression--docs`. Enter `sen(x)` in one of the input fields. The word `sen` should be displayed in an upright font, indicating that it is an operator. `sin`, `cos`, `tan` etc. should also be displayed in an upright font.
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
Size Change: +352 B (0%) Total Size: 833 kB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me-- but where does the list of AutoOperatorNames come from? Do we need to anglicize more of those?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #921 +/- ##
==========================================
+ Coverage 59.37% 60.73% +1.35%
==========================================
Files 417 419 +2
Lines 99226 99294 +68
Branches 6017 8582 +2565
==========================================
+ Hits 58916 60306 +1390
+ Misses 40310 38988 -1322
... and 72 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
@nedredmond good question - I should add a comment explaining that. The other names in the list are the defaults for MathQuill. The list is documented (sort of) here: https://docs.mathquill.com/en/latest/Config/#autooperatornames. That documentation links to the "Short Math Guide". The link is broken, but you can find a copy here: https://tug.ctan.org/info/short-math-guide/short-math-guide.pdf. The operators are in section 3.15. |
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (a13ec17) and published it to npm. You Example: yarn add @khanacademy/perseus@PR921 |
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @khanacademy/[email protected] ### Minor Changes - [#921](#921) [`81b9a562`](81b9a56) Thanks [@benchristel](https://github.com/benchristel)! - Make the Expression widget treat `sen` as equivalent to `sin`. The spelling `sen` is used in Portuguese. ## @khanacademy/[email protected] ### Minor Changes - [#921](#921) [`81b9a562`](81b9a56) Thanks [@benchristel](https://github.com/benchristel)! - Make the Expression widget treat `sen` as equivalent to `sin`. The spelling `sen` is used in Portuguese. ### Patch Changes - Updated dependencies \[[`81b9a562`](81b9a56)]: - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`81b9a562`](81b9a56)]: - @khanacademy/[email protected] - @khanacademy/[email protected] Author: khan-actions-bot Reviewers: benchristel Required Reviewers: Approved By: benchristel Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage, ⏭️ Publish npm snapshot, ✅ Extract i18n strings (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: #922
## Summary: So basically: - These two PRs ([1](#921) and [2](Khan/webapp#19111)) helped add support for Portuguese trig functions when typed in the input. - This [PR](#1497) added support for the text of trig buttons to be translated, however the input would still update with "sin" if the "sen" button was pressed. - The PR being reviewed updates the input correctly; so if the "sen" button is pressed, the input is updated with "sen" How this works: - When a button is pressed, the keypad yells into the void that "SIN" was pressed - The keypad translator has a callback for updating the input when "SIN" is pressed - BEFORE: it would just updated the input with `sin()` - NOW: it looks for the translated version of "sin", checks it against known safe translations, and either updates it with a known safe translation or the default "sin" Issue: LEMS-1621 ## Test plan: Once the strings are translated: - Go to a Portuguese page with an expression widget - Go to a question that requires the "sin" button - Open the keypad - Note the button is "sen" instead of "sin" - Click the "sen" button - Note the input is updated with "sen()" - Fill out the correct answer and submit - Note the validator accepts "sen" instead of "sin" Author: handeyeco Reviewers: handeyeco, benchristel, anakaren-rojas Required Reviewers: Approved By: benchristel Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: #1538
Summary:
sen
is used instead ofsin
in Portuguese. This change allows theExpression widget to accept both spellings. We simply convert any
instances of
sen
tosin
when evaluating the learner's answer.Issue: https://khanacademy.atlassian.net/browse/LC-290
Test plan:
yarn storybook
.http://localhost:6006/?path=/docs/perseus-widgets-expression--docs
.Enter
sen(x)
in one of the input fields. The wordsen
should bedisplayed in an upright font, indicating that it is an operator.
sin
,cos
,tan
etc. should also be displayed in an upright font.