-
Notifications
You must be signed in to change notification settings - Fork 350
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
Fixup image caption overlap on mobile #765
Conversation
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
Size Change: +10 B (0%) Total Size: 852 kB
ℹ️ View Unchanged
|
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (4ad5bd9) and published it to npm. You Example: yarn add @khanacademy/perseus@PR765 |
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.
One question about image width, but otherwise, this looks good! Thanks for fixing.
<figure | ||
className="perseus-image-widget" | ||
style={{ | ||
maxWidth: backgroundImage.width, |
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.
question: Are we guaranteed to always have a width?
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.
Yes background image will always have a width, as far as I know
fd74dc2
to
4ad5bd9
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #765 +/- ##
==========================================
- Coverage 65.50% 61.22% -4.28%
==========================================
Files 481 499 +18
Lines 103961 109231 +5270
Branches 5620 8853 +3233
==========================================
- Hits 68095 66875 -1220
- Misses 35866 42356 +6490
... and 759 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Summary:
Fixes image caption overlapping on mobile and re-centers text and image so that it matches previous behavior.
After the fix:
Issue: https://khanacademy.atlassian.net/browse/LC-1351
Test plan:
This can be test a little bit in storybook:
However to really test this, it needs a ZND. After deploying a ZND check out