fix: tall image clipping on read alongs #501
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Goal?
This PR fixes the clipping of tall images, which sometimes occurs when text column of the read along is narrow.
The applied changes are to properly implement HTML nesting rules and CSS Grid rules which lets the browser infer width and height information on the parent container of the image.
HTML nesting rules states that inline elements like
<span />cannot contain block elements like<div />. The<img />tag had that hierarchy (html > body > ... > div > span > div > img)CSS Grid rules requires that grid cells must be defined directly underneath the grid parent container, here's an incorrect implementation:
Correcting both issues seems to have corrected the tall image clipping problem.
Fixes?
Closes #468
Feedback sought?
Validate my assumptions. I'm also not familiar with other ReadAlongs UI design decisions that this solution breaks.
Priority?
Medium, but it is a bug reported by users.
Tests added?
No, but tests still passes after restructuring the HTML.
How to test?
To recreate the broken behavior (in production for example):
With this fix:
Confidence?
Medium. As mentioned above, this seems to work but have I introduced breaking UI look and feel.
Version change?