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

Inequality producing empty canvases at root level on resize #1245

Merged
merged 3 commits into from
Dec 12, 2024

Conversation

jacbn
Copy link
Contributor

@jacbn jacbn commented Dec 6, 2024

If makeInequality is called with an undefined ref, p5 proceeds to generate a new canvas at root level with seemingly fullscreen sizing. With an adjacent change to inequality, this forces the element to always be defined and will quietly cancel the setup in the known cases when this occurs.

Copy link
Contributor Author

@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.

Is not required, but should be paired with the updated version of inequality that forces the element type to be defined.

Copy link

codecov bot commented Dec 6, 2024

Codecov Report

Attention: Patch coverage is 0% with 18 lines in your changes missing coverage. Please review.

Project coverage is 36.59%. Comparing base (4ff419a) to head (d2016ca).
Report is 18 commits behind head on master.

Files with missing lines Patch % Lines
...ponents/content/IsaacSymbolicChemistryQuestion.tsx 0.00% 3 Missing and 2 partials ⚠️
.../components/content/IsaacSymbolicLogicQuestion.tsx 0.00% 2 Missing and 2 partials ⚠️
...c/app/components/content/IsaacSymbolicQuestion.tsx 0.00% 1 Missing and 2 partials ⚠️
src/app/components/pages/Equality.tsx 0.00% 1 Missing and 2 partials ⚠️
...app/components/elements/modals/inequality/utils.ts 0.00% 1 Missing and 1 partial ⚠️
...nts/elements/modals/inequality/InequalityModal.tsx 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1245      +/-   ##
==========================================
- Coverage   36.62%   36.59%   -0.03%     
==========================================
  Files         451      451              
  Lines       19763    19779      +16     
  Branches     6510     6504       -6     
==========================================
  Hits         7238     7238              
- Misses      11880    11886       +6     
- Partials      645      655      +10     

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

@jacbn jacbn marked this pull request as ready for review December 9, 2024 11:35
@sjd210
Copy link
Contributor

sjd210 commented Dec 11, 2024

What strange behaviour from p5 :/ Anyway, this certainly seems to fix it and better typing is always good regardless 👍

@sjd210 sjd210 merged commit 346c713 into master Dec 12, 2024
4 checks passed
@sjd210 sjd210 deleted the hotfix/inequality-targeting-wrong-element branch December 12, 2024 09:52
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