Skip to content

[Image] | (UX) | Don't upscale images within explore modal if size is unknown#3413

Merged
nishasy merged 4 commits intomainfrom
image-no-size-modal-broken
Mar 30, 2026
Merged

[Image] | (UX) | Don't upscale images within explore modal if size is unknown#3413
nishasy merged 4 commits intomainfrom
image-no-size-modal-broken

Conversation

@nishasy
Copy link
Copy Markdown
Contributor

@nishasy nishasy commented Mar 24, 2026

Summary:

Behind our image-widget-upgrade-scale feature flag, if an image is an SVG, it gets scaled up at least 2x. In the case that the SVG was saved without an image, this makes the image become too big and expand beyond the size of the modal. (In the case that we do know the size of the image, this is accounted for when we divide by the scale in the height calculation.)

Fixing this by using scale=1 for all cases in which the size in unknown.

Issue: LEMS-4012

Test plan:

Storybook

  • /?path=/story/widgets-image-widget-demo--large-svg-image-with-no-size-saved-scale-flag
  • Regression test (no flag, but will be useful when we remove flag): /?path=/story/widgets-image-visual-regression-tests-interactions--long-description-clicked-state-with-no-size-large-svg-image
Before After
Screenshot 2026-03-26 at 1 09 08 PM Screenshot 2026-03-26 at 1 18 34 PM

@nishasy nishasy self-assigned this Mar 24, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 24, 2026

🗄️ Schema Change: No Changes ✅

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 24, 2026

Size Change: +14 B (0%)

Total Size: 494 kB

Filename Size Change
packages/perseus/dist/es/index.js 193 kB +14 B (+0.01%)
ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 20.5 kB
packages/keypad-context/dist/es/index.js 1 kB
packages/kmath/dist/es/index.js 6.21 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 11.9 kB
packages/perseus-core/dist/es/index.js 25.1 kB
packages/perseus-editor/dist/es/index.js 101 kB
packages/perseus-linter/dist/es/index.js 9.3 kB
packages/perseus-score/dist/es/index.js 9.66 kB
packages/perseus-utils/dist/es/index.js 403 B
packages/perseus/dist/es/strings.js 8.09 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
Copy Markdown
Contributor

github-actions bot commented Mar 24, 2026

🛠️ Item Splitting: No Changes ✅

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 24, 2026

npm Snapshot: Published

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

Example:

pnpm add @khanacademy/perseus@PR3413

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

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

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

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

@nishasy nishasy changed the title [not for review] extra cases in storybook for image widget [Image] | (UX) | Don't upscale images within explore modal if size is unknown Mar 26, 2026
@github-actions github-actions bot added the schema-change Attached to PRs when we detect Perseus Schema changes in it label Mar 26, 2026
…upscale images within explore modal if size is unknown
@github-actions github-actions bot removed the schema-change Attached to PRs when we detect Perseus Schema changes in it label Mar 26, 2026
@nishasy nishasy marked this pull request as ready for review March 26, 2026 20:20
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review.

Tip: disable this comment in your organization's Code Review settings.

Copy link
Copy Markdown
Member

@catandthemachines catandthemachines left a comment

Choose a reason for hiding this comment

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

Yay! Thanks for this fix!

@nishasy nishasy merged commit 06ac0a1 into main Mar 30, 2026
13 of 19 checks passed
@nishasy nishasy deleted the image-no-size-modal-broken branch March 30, 2026 22:23
anakaren-rojas pushed a commit that referenced this pull request Mar 31, 2026
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]

### Patch Changes

-   [#3429](#3429) [`41bea5fd10`](41bea5f) Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (UX) | Stop large images from overflowing


-   [#3413](#3413) [`06ac0a160d`](06ac0a1) Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (UX) | Don't upscale images within explore modal if size is unknown


-   [#3426](#3426) [`dae8b1d931`](dae8b1d) Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (a11y) | Update zoom view SRUX

## @khanacademy/[email protected]

### Patch Changes

-   Updated dependencies \[[`41bea5fd10`](41bea5f), [`06ac0a160d`](06ac0a1), [`dae8b1d931`](dae8b1d)]:
    -   @khanacademy/[email protected]

Author: khan-actions-bot

Reviewers: claude[bot], anakaren-rojas

Required Reviewers:

Approved By: anakaren-rojas

Checks: ⏭️  2 checks have been skipped, ✅ 6 checks were successful

Pull Request URL: #3430
catandthemachines pushed a commit that referenced this pull request Apr 1, 2026
… unknown (#3413)

## Summary:
Behind our `image-widget-upgrade-scale` feature flag, if an image is an SVG, it gets scaled up at least 2x. In the case that the SVG was saved without an image, this makes the image become too big and expand beyond the size of the modal. (In the case that we do know the size of the image, this is accounted for when we divide by the scale in the `height` calculation.)

Fixing this by using `scale=1` for all cases in which the size in unknown.

Issue: [LEMS-4012](https://khanacademy.atlassian.net/browse/LEMS-4012)

## Test plan:
Storybook
- `/?path=/story/widgets-image-widget-demo--large-svg-image-with-no-size-saved-scale-flag`
- Regression test (no flag, but will be useful when we remove flag): `/?path=/story/widgets-image-visual-regression-tests-interactions--long-description-clicked-state-with-no-size-large-svg-image`

| Before | After |
| --- | --- |
| <img width="1723" height="773" alt="Screenshot 2026-03-26 at 1 09 08 PM" src="https://github.com/user-attachments/assets/fad80bba-8749-41f1-9300-461bc25f171b" /> | <img width="1398" height="718" alt="Screenshot 2026-03-26 at 1 18 34 PM" src="https://github.com/user-attachments/assets/5734bfc0-1464-42d5-b6de-0bfd56a5d04e" /> |

[LEMS-4012]: https://khanacademy.atlassian.net/browse/LEMS-4012?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

Author: nishasy

Reviewers: claude[bot], catandthemachines, ivyolamit

Required Reviewers:

Approved By: catandthemachines

Checks: ⏭️  1 check has been skipped, ✅ 10 checks were successful

Pull Request URL: #3413
catandthemachines pushed a commit that referenced this pull request Apr 1, 2026
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]

### Patch Changes

-   [#3429](#3429) [`41bea5fd10`](41bea5f) Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (UX) | Stop large images from overflowing


-   [#3413](#3413) [`06ac0a160d`](06ac0a1) Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (UX) | Don't upscale images within explore modal if size is unknown


-   [#3426](#3426) [`dae8b1d931`](dae8b1d) Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (a11y) | Update zoom view SRUX

## @khanacademy/[email protected]

### Patch Changes

-   Updated dependencies \[[`41bea5fd10`](41bea5f), [`06ac0a160d`](06ac0a1), [`dae8b1d931`](dae8b1d)]:
    -   @khanacademy/[email protected]

Author: khan-actions-bot

Reviewers: claude[bot], anakaren-rojas

Required Reviewers:

Approved By: anakaren-rojas

Checks: ⏭️  2 checks have been skipped, ✅ 6 checks were successful

Pull Request URL: #3430
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