fix(Mime): do not set natural dimensions when embedded - #3338
Open
jospoortvliet wants to merge 1 commit into
Open
fix(Mime): do not set natural dimensions when embedded#3338jospoortvliet wants to merge 1 commit into
jospoortvliet wants to merge 1 commit into
Conversation
components (Images.vue, Videos.vue, Audios.vue) are mounted
outside of the Viewer's own modal by other apps, e.g. the Files app's
ReferenceFileWidget for Smart Picker previews. updateHeightWidth()
detected \"inside the modal\" by searching for a .modal-wrapper in the
parent DOM; anywhere else it fell through to natural image/video size,
producing hardcoded inline dimensions such as
style=\"height: 1912px; width: 2940px\" that get clipped by the
widget's overflow: hidden container.
Declare the isEmbedded prop (already passed as is-embedded by
ReferenceFileWidget.vue but previously swallowed into \$attrs) and
return early from updateHeightWidth() when set, leaving height/width
unset. Images.vue's imgStyle must return {} in that case too, since
Math.round(null * zoomRatio) would otherwise emit
style=\"height: 0px; width: 0px\" and blank the widget. Let CSS size
the element instead via a new .embedded modifier using
object-fit: contain, so the aspect ratio is preserved without
upscaling small images.
The modal path (.modal-wrapper found) and the existing \"natural
dimensions not yet measured\" fallback are unchanged.
Assisted-by: Claude Sonnet 5:claude-sonnet-5
Signed-off-by: Jos Poortvliet <jospoortvliet@gmail.com>
Member
Author
|
just to check - here too - should I recompile and commit assets, or does CI do that?
|
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.
When using the Smart Picker to view an image, the size of these images is shared with the app, and both Talk and Text take try and display them as such. Now Talk has a different CSS that kind of limits the damage, but it doesn't look great either. Screenshots:
BEFORE: Talk will basically make it fit, distorting the image:

AFTER: No more distortion!

BEFORE: Text just crops (the image on the bottom is the link to the same file as the attachment above it):

AFTER: Looks just fine, can even be resized.

BEFORE: In Text, Tables are entirely broken by inserting an image, as it's rendered 1:1 size and thus creates a huge horizontal scroll.

AFTER: fits nicely in a table cell;

Makes images a lot more useful this way. It does not impact the viewer modal at all - checked that from Talk, Files and Text.
The cause
To let Claude explain the cause of this:
Mime.js'supdateHeightWidth()decided whether it was running inside the modal by searching the parent DOM for.modal-wrapper. Anywhere else, it fell through to natural image/video size and wrote that out as a hardcoded inline style, e.g.style="height: 1912px; width: 2940px". The widget's container hasoverflow: hidden, so only the top-left corner of the image is visible, and in a table cell the oversized intrinsic width blows out the column.Change
This is what was changed (Claude's words);
isEmbeddedprop on theMimemixin. The Files widget already passesis-embedded="true"to the handler component, but nothing declared the prop, so it was silently swallowed into$attrs.updateHeightWidth()whenisEmbeddedis set, leavingheight/widthunset. The existingelsebranch (natural dimensions not yet measured, inside the modal) is untouched.Images.vue'simgStylereturns{}when embedded, instead of computingMath.round(null * zoomRatio)→0, which would otherwise renderstyle="height: 0px; width: 0px"and blank the widget entirely..embeddedCSS modifier (max-width/max-height: 100%,object-fit: contain) so the browser sizes the element instead, preserving aspect ratio without upscaling small images.Applications can work around this bug in their own CSS (Text, Talk), but that's less than ideal.
Testing
npm run lintandnpm run stylelintpass.🤖 AI (if applicable)
Assisted-by: Claude Sonnet 5:claude-sonnet-5
Frankly, Claude did the code/analysis. I tested etc...