The height: auto is absolutely necessary, otherwise the height=xyz from the IMG tag will be used and is then distorted.

I have tested this item ? unsuccessfully on e674317

The height: auto is absolutely necessary, otherwise the height=xyz from the IMG tag will be used and is then distorted.

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/39766.

I'm a bit confused. Are you requesting me change the height to auto? I never said anything about height, and my commit was only setting max width.

The height: auto is absolutely necessary, otherwise the height=xyz from the IMG tag will be used and is then distorted.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/39766.

I'm a bit confused. Are you requesting me change the height to auto? I never said anything about height, and my commit was only setting max width.

The PR for max-width ist successful, but if you take an high-edgewise image then it will be warped.

That would be the case if we were setting width, but in this case we are setting max width to ensure images that are too wide for the login box don't go outside the login box. The height style that is set is already in core.

max-width solves the problem for images that are too wide for the login module. If your logo is portrait or square and maybe not so wide then the max-width has no effect, but the max-height and the image looks distorted. Normally it should be max-width:100% in combination with height:auto or width: auto and max-height: 4.4rem to allow the image to resize correctly.

You can't combine width, max-width and max-height...

Here some screenshots with different CSS definitions (image size 419 x 542px):

max-height: 4.4rem;

max-width:100%;
height: auto;

max-height: 4.4rem;
max-width: 100%;

max-height: 4.4rem;
width: auto;

I think, in the case of the logo for the login module, the height of the image is more important than the width. Imagine if someone puts a 800px high image, the login will looks very weird and you will need to scroll to be able to enter your credentials.
IMHO is max-height: 4.4rem and width: auto the better solution... but that should be tested with different kind of logos.

Agreed

Not sure if object-fit would help here somehow.

Not sure if object-fit would help here somehow.

I was thinking about that too, but in this case you want to display the full logo, object-fit will crop the image...

Took a high picture on purpose. Loaded: 525 x 700 px. :-) But there is no new npm package yet for testing.

Would above example OK for patch, @drmenzelit?

I have tested this item ✅ successfully on 7e5315e

I have tested this item ✅ successfully on 7e5315e

RTC

Thank you for fixing that!

Fix works, but can I suggest a slightly different approach? 4.4em is tiny for a lot of logos.

What about something like:

max-height: 15rem;
max-width: 100%;
object-fit: contain;

This bounds the logo by whichever dimension is larger and makes sure it doesn't get distorted, and also makes sure it's not way too tall or too short.

Sorry, I didn't catch this before it was merged!