Repository navigation
fix: fit inline images in their square and centre them on the cap height - #1157
Merged
Merged
Conversation
2 of 6 tasks
maksg
approved these changes
Nov 11, 2026
maksg
left a comment
Collaborator
There was a problem hiding this comment.
Looks solid! 💯
Left just one small comment
|
|
||
| - (void)testInlineImageIsCentredOnTheCapHeight | ||
| { | ||
| for (NSArray<NSString *> *item in @[ @[ @"HH {image} HH", @"" ], @[ @"# HH {image} HH", @"" ] ]) { |
Collaborator
There was a problem hiding this comment.
Both cases pass @"" for strongFontFamily:, so nothing tests the bold path or ENRMImageAttachmentsAdoptTextFonts: the suite still passes with that call removed. A **…** case in this loop wouldn't catch it either, because Georgia's cap height is too close to the system font's (the centre moves 0.1pt, the tolerance is 0.5).
A direct check does:
- (void)testInlineImageTakesTheFontOfStrongText
{
ENRMImageAttachment *image = nil;
NSUInteger index = 0;
NSTextStorage *storage = [self render:@"**HH {image} HH**"
imageSize:CGSizeMake(32, 32)
strongFontFamily:@"Georgia"
image:&image
index:&index];
UIFont *textFont = [storage attribute:NSFontAttributeName atIndex:index + 2 effectiveRange:NULL];
XCTAssertEqualObjects(textFont.familyName, @"Georgia");
XCTAssertEqualObjects(image.textFont, textFont);
}It fails without the ENRMImageAttachmentsAdoptTextFonts(output) call. The @"" entries and the ternary in this loop can then go.
hryhoriiK97
approved these changes
Nov 12, 2026
hryhoriiK97
left a comment
Collaborator
There was a problem hiding this comment.
LGTM! Thank you for the fix!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What/Why?
Fixes #1083. An inline image (
after text) looked different on each platform. iOS and web stretched a non-square image to fill theinlineImage.sizesquare. iOS also missed the cap height, more so the larger the image, and web centred onvertical-align: middle, below the capitals. Android already fitted the image inside the square and centred it on the cap height (#1089).Every platform now fits the image inside the square, keeping its proportions, and centres it on the cap height of the text around it. The issue suggested filling the square (cover). This PR uses contain instead, so nothing is ever cropped. That's what Android already did, and what chat apps with a fixed emoji-sized square do, Discord for one.
contain/.fit. The offset uses the font of the text around the image, bold and italic included. On native iOS a paragraphlineHeightmoves the image together with the text.object-fit: containandvertical-align: calc(0.5cap - size / 2), falling back tomiddlewhere thecapunit isn't supported (Safari before 17.2).enriched-markdown-android):image.borderRadiuswas converted to pixels twice, so a rounded 20 dp inline image came out as a circle.Testing
Open Storybook › EnrichedMarkdownText › Inline › Inline Image › Non Square in the example app. It shows wide, tall, square, 10:1 and 1:10 images at
inlineImage.size: 48. The web screenshots are the same Markdown in the web example.Android didn't change, and now matches iOS:
InlineImagePlacementTests(native iOS, measures painted pixels),ENRMInlineImagePlacementTests(React Native iOS),inlineImageStyle.test.ts(web), andImageCornerRadiusTestplus a wide image case inInlineImagePlacementTest(Android).testDebugUnitTestwithktlintCheckin both Android packages.inline_image_displaybaselines (Android, iOS, native iOS). I haven't run the full suite.PR Checklist
inline_image_displaywas run