-
Notifications
You must be signed in to change notification settings - Fork 359
[Explanation] Bugfix - Math within the widget has additional space added below it when in mobile viewport #3084
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
base: main
Are you sure you want to change the base?
Conversation
…ion-widget] docs(changeset): [Explanation] Bugix - Math within the widget has additional space added below it when in mobile viewport
🗄️ Schema Change: No Changes ✅ |
|
Size Change: +92 B (+0.02%) Total Size: 499 kB
ℹ️ View Unchanged
|
🛠️ Item Splitting: No Changes ✅ |
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (ec0f963) and published it to npm. You Example: pnpm add @khanacademy/perseus@PR3084If you are working in Khan Academy's frontend, you can run the below command. ./dev/tools/bump_perseus_version.ts -t PR3084If you are working in Khan Academy's webapp, you can run the below command. ./dev/tools/bump_perseus_version.js -t PR3084 |
…ion-widget] Add logic to exclude invalid measurements when parent height is zero. Add debugging statements to aid in verifying calculations.
…ion-widget] Additional debugging messages.
…ion-widget] Use ancestor to determine if width is zero - parent node can still have width when hidden.
…ion-widget] Set initial state to "expanded" to test logic.
…ion-widget] Reset initial state of explanation widget. Remove debugging messages.
…ion-widget] Add a height check on the children of the math container to ensure that we are getting the best representation of the math height.
…ion-widget] Add a default value for the text default height.
ivyolamit
left a comment
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.
@mark-fitzgerald implementation looks logical to me, thanks for making this change 🎉
Summary:
When TeX is displayed in a mobile viewport, it is reduced in size so that it can be seen in its entirety. It is made "zoomable" so that a user can click the math and have it display at its normal size, and scroll to see the offscreen content. The math is initially reduced in size by calculating the available width and comparing it to the actual width of the math element.
When zoomable math is displayed within the Explanation widget, it is hidden along with the rest of the explanation content. Part of the hiding involves reducing the size of the explanation container to a width of zero. When an HTML element has a width of zero, it causes the height to be extremely tall in order to accommodate all the content within. When the zoomable math reduction is calculated in this situation, it uses this invalid height information. This PR resolve this by using more reliable height information (elements not affected by the zero-width container). It only does this adjusted calculation when one of the ancestor containers has
aria-hidden="true"added (indicating that the element is hidden from view).Issue: LEMS-3608
Test plan:
Bugfix:
Regression test: