Skip to content

Include zoom level in unlimted interactive graph calculations #3221

Open
SonicScrewdriver wants to merge 5 commits intomainfrom
ok-zoomer
Open

Include zoom level in unlimted interactive graph calculations #3221
SonicScrewdriver wants to merge 5 commits intomainfrom
ok-zoomer

Conversation

@SonicScrewdriver
Copy link
Contributor

@SonicScrewdriver SonicScrewdriver commented Jan 28, 2026

Summary:

This PR fixes a bug related to our Mobile Application and our Interactive Graph Widgets.

The issue was that we're adding a zoom style in frontend based on the user's fontScale, in order to help support accessibility features on iOS. Unfortunately, this zoom affects coordinate calculations for click/touch events, as both clientX/clientY and getBoundingClientRect() return zoomed values, but the SVG coordinate system expects unzoomed pixel values. As a result, iOS users with increased fontSizes would find the position of points they add/drag to be inaccurate to their touch position.

In order to resolve that, I've added logic to calculate the cumulative zoom level. I chose to do it cumulatively, rather than finding a specific element, to help prevent future regressions. It seemed plausible that class names or hierarchy could be changed at some point in the future, and the cumulative approach allows us to remain platform agnostic. The calculations seemed fairly negligible, but I'm happy to adjust if anyone has any concerns about it!

Issue: LEMS-3258

Test plan:

  • Manual Testing using fontScale parameter on Desktop
  • Manual Testing with Mobile Applications (Both Android and iOS, at both normal and enhanced zooms)
  • Existing tests pass

@github-actions
Copy link
Contributor

github-actions bot commented Jan 28, 2026

🗄️ Schema Change: No Changes ✅

@github-actions
Copy link
Contributor

github-actions bot commented Jan 28, 2026

Size Change: +192 B (+0.04%)

Total Size: 485 kB

Filename Size Change
packages/perseus/dist/es/index.js 187 kB +192 B (+0.1%)
ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 20.8 kB
packages/keypad-context/dist/es/index.js 1 kB
packages/kmath/dist/es/index.js 5.98 kB
packages/math-input/dist/es/index.js 98.5 kB
packages/math-input/dist/es/strings.js 1.61 kB
packages/perseus-core/dist/es/index.item-splitting.js 12.1 kB
packages/perseus-core/dist/es/index.js 25 kB
packages/perseus-editor/dist/es/index.js 99.2 kB
packages/perseus-linter/dist/es/index.js 8.83 kB
packages/perseus-score/dist/es/index.js 9.32 kB
packages/perseus-utils/dist/es/index.js 403 B
packages/perseus/dist/es/strings.js 7.44 kB
packages/pure-markdown/dist/es/index.js 1.39 kB
packages/simple-markdown/dist/es/index.js 6.71 kB

compressed-size-action

@github-actions
Copy link
Contributor

github-actions bot commented Jan 28, 2026

🛠️ Item Splitting: No Changes ✅

@github-actions
Copy link
Contributor

github-actions bot commented Jan 28, 2026

npm Snapshot: Published

Good news!! We've packaged up the latest commit from this PR (098f771) and published it to npm. You
can install it using the tag PR3221.

Example:

pnpm add @khanacademy/perseus@PR3221

If you are working in Khan Academy's frontend, you can run the below command.

./dev/tools/bump_perseus_version.ts -t PR3221

If you are working in Khan Academy's webapp, you can run the below command.

./dev/tools/bump_perseus_version.js -t PR3221

@SonicScrewdriver SonicScrewdriver changed the title Initial test of grabbing the zoom level to resolve iOS bug Include zoom level in unlimted interactive graph calculations Jan 29, 2026
@SonicScrewdriver SonicScrewdriver marked this pull request as ready for review January 29, 2026 19:20
* @param element - The DOM element to check for CSS zoom
* @returns The cumulative zoom factor (e.g., 1.5 for 150% zoom, 1.0 for no zoom)
*/
export function getCSSZoomFactor(element: Element): number {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've kept this util in Interactive Graph for now, but I suppose it's possible that I could find instances where the zoom level is affecting the behaviour of other widgets on iOS.

If I find any such cases, I'll move this logic to a more central location. Until then, I think it makes sense here.

Copy link
Contributor

@mark-fitzgerald mark-fitzgerald left a comment

Choose a reason for hiding this comment

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

Good find for the computed styling that impacts this widget!! I wonder if this could be used for Radio widget fade bars.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants