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

Add front-end warnings and Inequality integration for new Chemistry options #1165

Merged
merged 13 commits into from
Oct 25, 2024

Conversation

sjd210
Copy link
Contributor

@sjd210 sjd210 commented Oct 3, 2024

Adds option for coefficient scaling:

  • e.g. 2 H2 + O2 -> 2 H2O ===>>> H2 + 1/2 O2 -> H2O

and for disabling certain "available symbols" such as is done with trig functions for Symbolic Maths questions.

  • e.g. _state_symbols controls (s), (l), (g), (aq)

Unavailable symbols are removed from the Inequality modal menu. If state symbols are unavailable and the user tries to use one in text-input, they are warned.


Both of these are not used as options for Nuclear Physics questions, although the logic is applied from available symbols to disallow state symbols, brackets, equilibrium arrows, and dots from its Inequality menu.


Also fixes a minor issue where processing the _no_alphabet meta symbol would skip the next meta symbol from being processed. The only place I could find this impacting was here (note the "_trigs" in the available symbols list).

Copy link

codecov bot commented Oct 3, 2024

Codecov Report

Attention: Patch coverage is 1.29870% with 76 lines in your changes missing coverage. Please review.

Project coverage is 36.44%. Comparing base (2d98a71) to head (fdcb4a1).
Report is 26 commits behind head on master.

Files with missing lines Patch % Lines
...ponents/content/IsaacSymbolicChemistryQuestion.tsx 0.00% 34 Missing ⚠️
...app/components/elements/modals/inequality/utils.ts 0.00% 26 Missing ⚠️
...nts/elements/modals/inequality/InequalityModal.tsx 0.00% 10 Missing ⚠️
src/app/services/questions.ts 16.66% 5 Missing ⚠️
...components/elements/modals/inequality/constants.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1165      +/-   ##
==========================================
- Coverage   36.46%   36.44%   -0.03%     
==========================================
  Files         435      435              
  Lines       19290    19308      +18     
  Branches     6348     6344       -4     
==========================================
+ Hits         7035     7036       +1     
- Misses      11617    12234     +617     
+ Partials      638       38     -600     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sjd210 sjd210 marked this pull request as draft October 4, 2024 10:22
Copy link
Contributor

@jacbn jacbn left a comment

Choose a reason for hiding this comment

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

I can't speak for what categories content would like in each section so some of the testing here is best left to them. The code is looking good though!

@@ -89,7 +90,7 @@ export function generateMathsBasicFunctionsItems(): MenuItemProps[] {
];
}

export function generateMathsTrigFunctionItem(name: string): MenuItemProps {
export function generateMathsTrigFunctionItem(name: string): MenuItemProps { // ohh heck
Copy link
Contributor

Choose a reason for hiding this comment

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

this is the general vibe inequality gives off...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😔 You're making me tempted to keep it

@jacbn jacbn merged commit c3b65e1 into master Oct 25, 2024
4 checks passed
@jacbn jacbn deleted the improvement/new-chemistry-options branch October 25, 2024 13:13
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.

2 participants