Skip to content

[Image] | (UX) | Stop large images from overflowing#3429

Merged
nishasy merged 3 commits intomainfrom
large-images-overflowing
Mar 30, 2026
Merged

[Image] | (UX) | Stop large images from overflowing#3429
nishasy merged 3 commits intomainfrom
large-images-overflowing

Conversation

@nishasy
Copy link
Copy Markdown
Contributor

@nishasy nishasy commented Mar 30, 2026

Summary:

I noticed that large images without a size overflow past the bounds (article).
Note that this case is currently only possible behind the scale feature flag
(the non-flag implementation always finds a size for the image).

Fixing that here by giving the image loader a max width of 100% to prevent it
from growing larger than its container.

Issue: none

Test plan:

  • /?path=/story/widgets-image-widget-demo--large-image-with-no-size-saved-scale-flag
  • /?path=/story/widgets-image-widget-demo--small-image-with-no-size-saved-scale-flag
Before After
large large
Screenshot 2026-03-30 at 2 32 22 PM Screenshot 2026-03-30 at 2 32 43 PM
small small
Screenshot 2026-03-30 at 2 32 30 PM Screenshot 2026-03-30 at 2 32 52 PM

nishasy added 3 commits March 30, 2026 14:37
…erflowing

I noticed that large images without a size overflow past the bounds.
Note that this case is currently only possible behind the scale feature flag
(the non-flag implementation always finds a size for the image).

Fixing that here by giving the image loader a max width of 100% to prevent it
from growing larger than its container.

Issue: none

Test plan:
- `/?path=/story/widgets-image-widget-demo--large-image-with-no-size-saved-scale-flag`
- `/?path=/story/widgets-image-widget-demo--small-image-with-no-size-saved-scale-flag`
@nishasy nishasy self-assigned this Mar 30, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 30, 2026

🗄️ Schema Change: No Changes ✅

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 30, 2026

Size Change: 0 B

Total Size: 494 kB

ℹ️ 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/index.js 192 kB
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

npm Snapshot: Published

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

Example:

pnpm add @khanacademy/perseus@PR3429

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

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

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

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

backgroundImage: {url: frescoImage.url},
alt: "Fresco painting",
},
};
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This regression story doesn't have the scale flag, but it should make sure that nothing changes after the scale flag is removed.

@nishasy nishasy marked this pull request as ready for review March 30, 2026 21:48
@github-actions
Copy link
Copy Markdown
Contributor

🛠️ Item Splitting: No Changes ✅

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 and subscribe this PR to future pushes, or @claude review once for a one-time 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.

These CSS changes look good to me. Thanks Nisha!

@nishasy nishasy merged commit 41bea5f into main Mar 30, 2026
16 of 25 checks passed
@nishasy nishasy deleted the large-images-overflowing branch March 30, 2026 22:25
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
## Summary:
I noticed that large images without a size overflow past the bounds ([article](https://www.khanacademy.org/science/ms-physics/x1baed5db7c1bb50b:teacher-resources/x1baed5db7c1bb50b:unit-guides/a/middle-school-physics-unit-guides)).
Note that this case is currently only possible behind the scale feature flag
(the non-flag implementation always finds a size for the image).

Fixing that here by giving the image loader a max width of 100% to prevent it
from growing larger than its container.

Issue: none

## Test plan:
- `/?path=/story/widgets-image-widget-demo--large-image-with-no-size-saved-scale-flag`
- `/?path=/story/widgets-image-widget-demo--small-image-with-no-size-saved-scale-flag`

| Before | After |
| --- | --- |
| large | large |
| <img width="1727" height="1044" alt="Screenshot 2026-03-30 at 2 32 22 PM" src="https://github.com/user-attachments/assets/632716f4-6024-4029-a92b-1351a5df42c8" /> | <img width="1727" height="1045" alt="Screenshot 2026-03-30 at 2 32 43 PM" src="https://github.com/user-attachments/assets/be747285-0f00-491a-b485-7aa12ed6f0ea" /> |
| small | small |
| <img width="1728" height="1043" alt="Screenshot 2026-03-30 at 2 32 30 PM" src="https://github.com/user-attachments/assets/a120379d-f31b-439c-9bcc-83fc696341ac" /> | <img width="1726" height="1044" alt="Screenshot 2026-03-30 at 2 32 52 PM" src="https://github.com/user-attachments/assets/4b8e12e2-3b4a-464a-aea0-ae2253ef1d5b" /> |

Author: nishasy

Reviewers: nishasy, claude[bot], catandthemachines, mark-fitzgerald, ivyolamit

Required Reviewers:

Approved By: catandthemachines

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

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