All images height issue #25665

abrarpathan19 · 2019-11-20T10:33:38Z

Description (*)

Fixed Issues (if relevant)

Since 2.3.3 Store logo automatic resize on mobile/smaller displays does not respect aspect ratio #25043: Since 2.3.3 Store logo automatic resize on mobile/smaller displays does not respect aspect ratio

Manual testing scenarios (*)

Magento 2.3.3
Upload images from backend (You can upload logo or images from cms block)
You can able to see the stretching of all Frontend images.

In Magento 2.3.3 latest version there is an image height issue, It was proper working in the previous version so I have done changes as per the previous version.

Questions or comments

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds are green)

update magento on 14-feb

m2-assistant · 2019-11-20T10:33:41Z

Hi @abrarpathan19. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

ihor-sviziev · 2019-11-20T15:26:24Z

@abrarpathan19,
Please sign CLA, otherwise we can't accept your Pull Request.

Update: We already have fix for this issue that was already tested: #25623

I'm closing your PR as duplicate.

If you'll have any questions or comments - feed free to write comment and/or re-open this PR.

m2-assistant · 2019-11-20T15:29:24Z

Hi @abrarpathan19, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

abrarpathan19 · 2019-11-21T05:32:38Z

@abrarpathan19,
Please sign CLA, otherwise we can't accept your Pull Request.

Update: We already have fix for this issue that was already tested: #25623

I'm closing your PR as duplicate.

If you'll have any questions or comments - feed free to write comment and/or re-open this PR.

@ihor-sviziev

I have signed the CLI.

In Magento 2.3.3 latest version there is an image height issue not even in the logo but entire store images.

The reason behind this PR #25623 is already fixed that I agree but that changes are only for logo, I have fixed that changes for the entire Magento frontend. you can compare my code and his code.

#25623 issue is fixed only for the Logo image. and my changes are for all images which are used in the frontend area in the store

ihor-sviziev · 2019-11-21T07:29:32Z

Hi @abrarpathan19,
So I believe there should be another steps to reproduce, not related to logo.
Could you describe them?

abrarpathan19 · 2019-11-21T07:34:04Z

Hello @ihor-sviziev,

Other images also work the same as the logo image is behaving now. and if we apply my changes we don't need to apply #25623 these changes.

Thanks :)

krzksz · 2019-11-21T17:41:14Z

This PR reverts the fix that was done inside d0ad63c, which makes me thing that maybe img should be separated from object, video, embed declaration and applying this change will bring back the bug with videos.

krzksz · 2019-11-21T19:00:55Z

lib/web/css/source/lib/_resets.less

@@ -55,7 +55,7 @@
    object,
    video,
    embed {
-        max-height: 100%;
+        height: auto;


As mentioned in the comment, this change was a part of d0ad63c and reverting it would most probably break something else.

My suggestion would be to split img from this bunch and also remove the style that was added for logo if we can have it globally.

Hello @krzksz,
Thanks for your suggestions. I have review your comment and agree with the same. As per your suggestion, I have added sperate property for img, please review

lib/web/css/source/lib/_resets.less

…agento2 into all-images-issue

engcom-Bravo · 2019-11-22T15:05:18Z

Hello @abrarpathan19. Thank You for contribution. Please watch the gifs below:

2.3-develop

Current PR branch

It is a Product Page being resized. Though I may be wrong, but as to me it looks like the behavior is pretty much the same for both cases. Could You please give us some steps to reproduce following which we would catch the evident difference between the behavior on develop branch and on the one with Your PR. Thank You.

krzksz · 2019-11-24T18:39:37Z

@magento give me 2.3-develop instance

magento-engcom-team · 2019-11-24T18:39:42Z

Hi @krzksz. Thank you for your request. I'm working on Magento 2.3-develop instance for you

magento-engcom-team · 2019-11-24T18:40:19Z

Hi @krzksz, here is your Magento instance.
Admin access: https://i-25665-2-3-develop.instances.magento-community.engineering/admin
Login: admin Password: 123123q
Instance will be terminated in up to 3 hours.

abrarpathan19 · 2019-11-25T05:05:58Z

Hello @abrarpathan19. Thank You for contribution. Please watch the gifs below:

2.3-develop

Current PR branch

It is a Product Page being resized. Though I may be wrong, but as to me it looks like the behavior is pretty much the same for both cases. Could You please give us some steps to reproduce following which we would catch the evident difference between the behavior on develop branch and on the one with Your PR. Thank You.

@engcom-Bravo,

Thanks for the update, Yes, I know the output will be the same but can you please review the CSS style are changed. Before it was fixed only for the logo (luma and Black only). but I have fixed it for entire store images (globally). I have already explained to Mateusz Krzeszowiak.

krzksz · 2019-11-25T06:33:00Z

Hey @abrarpathan19, can you give the example of an image in the shop that would be scaled incorrectly without your change? If not, then it means is is not really needed, right?

abrarpathan19 · 2019-11-25T07:50:18Z

Hey @abrarpathan19, can you give the example of an image in the shop that would be scaled incorrectly without your change? If not, then it means is is not really needed, right?

Hello @krzksz,

Regarding example please review the provided demo from the engcom-Bravo, It is behaving the same in the Current PR branch and 2.3-develop.

In Branch 2.3-develop following issue already merges and it was fixed only for the logo but I have fixed it for the whole store that is why we are removing these changes. whether you upload logos and any images from the admin.
#25623

Thanks

engcom-Bravo · 2019-11-25T15:01:24Z

@abrarpathan19, Probably I do not entirely understand You, but please take a look at the gifs once more. I intentionally put for a comparison two kinds of image: Logo image and Product Page image. They both behave correctly on the latest develop. Could You please give us an example where image would be resized better with Your PR than it is resized on the current develop version. Thank You.

gwharton · 2019-11-25T19:32:42Z

The error was first introduced in MAGETWO-59978 d0ad63c#diff-9cc331ae9c8d2d3be0f5607cfd3c570c. It causes ALL images (well possibly all but product images. Try putting an image in the footer instead of using product image, or create a cms page with an image in it. These will all resize incorrectly in 2.3-develop) to be resized incorrectly.

The fix in #25623 only fixed it for the logo images, as that was what the bug reported, e.g Problems resizing logo images.

This PR reverts the changes made in #25623 and re-introduces a new fix for MAGETWO-59978 that only affects video objects, and leaves the styles for img's working correctly.

This PR is the correct resolution IMHO.

krzksz · 2019-11-25T20:05:47Z

@magento give me 2.3-develop instance

magento-engcom-team · 2019-11-25T20:06:02Z

Hi @krzksz. Thank you for your request. I'm working on Magento 2.3-develop instance for you

magento-engcom-team · 2019-11-25T20:06:39Z

Hi @krzksz, here is your Magento instance.
Admin access: https://i-25665-2-3-develop.instances.magento-community.engineering/admin
Login: admin Password: 123123q
Instance will be terminated in up to 3 hours.

krzksz · 2019-11-25T20:25:07Z

The error was first introduced in MAGETWO-59978 d0ad63c#diff-9cc331ae9c8d2d3be0f5607cfd3c570c. It causes ALL images (well possibly all but product images. Try putting an image in the footer instead of using product image, or create a cms page with an image in it. These will all resize incorrectly in 2.3-develop) to be resized incorrectly.

The fix in #25623 only fixed it for the logo images, as that was what the bug reported, e.g Problems resizing logo images.

This PR reverts the changes made in #25623 and re-introduces a new fix for MAGETWO-59978 that only affects video objects, and leaves the styles for img's working correctly.

This PR is the correct resolution IMHO.
I totally agree it is a correct fix if the issue can be reproduced anywhere which I am not able to do: https://i-25665-2-3-develop.instances.magento-community.engineering/

For me the result seems correct both for images in CMS pages and static blocks with img styling from 2.3-develop.

abrarpathan19 · 2019-12-02T06:42:36Z

Hello @krzksz,

Do you need any update from my end?

Abrar

krzksz · 2019-12-02T06:45:21Z

Hi @abrarpathan19! Yes, please provide an information which image in which browser is not scaling for you properly without your fix. As you can see above we are struggling to find such image.

abrarpathan19 · 2019-12-02T07:15:12Z

@magento give me 2.3-develop instance

magento-engcom-team · 2019-12-02T07:15:14Z

Hi @abrarpathan19. Thank you for your request. I'm working on Magento 2.3-develop instance for you

magento-engcom-team · 2019-12-02T07:15:54Z

Hi @abrarpathan19, here is your Magento instance.
Admin access: https://i-25665-2-3-develop.instances.magento-community.engineering/admin
Login: admin Password: 123123q
Instance will be terminated in up to 3 hours.

abrarpathan19 · 2019-12-02T07:50:39Z

Hi @abrarpathan19! Yes, please provide an information which image in which browser is not scaling for you properly without your fix. As you can see above we are struggling to find such image.

Hello

Please review http://prntscr.com/q4w15r
https://i-25665-2-3-develop.instances.magento-community.engineering/

krzksz · 2019-12-02T07:53:38Z

@abrarpathan19 Can you give me any other example then the logo which is fixed on 2.3-develop?

VladimirZaets · 2019-12-16T19:28:48Z

@abrarpathan19, I am closing this PR now due to inactivity.
Please reopen and update if you wish to continue.
Thank you for collaboration

m2-assistant · 2019-12-16T19:28:51Z

Hi @abrarpathan19, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

abrarpathan19 added 3 commits February 14, 2019 13:43

Merge pull request #5 from magento/2.3-develop

545adc8

update magento on 14-feb

Merge remote-tracking branch 'upstream/2.3-develop' into 2.3-develop

187c6fd

Fixed-images-issue-resolve-Magento-233

c219b53

magento-engcom-team added Area: Lib/Frontend Release Line: 2.3 Partner: Krish TechnoLabs partners-contribution Pull Request is created by Magento Partner labels Nov 20, 2019

ihor-sviziev self-assigned this Nov 20, 2019

ihor-sviziev closed this Nov 20, 2019

abrarpathan19 reopened this Nov 21, 2019

ghost unassigned ihor-sviziev Nov 21, 2019

krzksz suggested changes Nov 21, 2019

View reviewed changes

ghost assigned krzksz Nov 21, 2019

ghost added the Progress: needs update label Nov 21, 2019

Update _resets.less

39b808c

krzksz suggested changes Nov 22, 2019

View reviewed changes

lib/web/css/source/lib/_resets.less Show resolved Hide resolved

abrarpathan19 added 5 commits November 22, 2019 15:02

Merge remote-tracking branch 'upstream/2.3-develop' into 2.3-develop

8db7eb9

Fixed-images-issue-resolve-Magento-233

fe61a67

Update _resets.less

c959917

remove-unuse-code

154df7c

Merge branch 'all-images-issue' of https://github.com/abrarpathan19/m…

6a4221a

…agento2 into all-images-issue

krzksz approved these changes Nov 22, 2019

View reviewed changes

ghost removed the Progress: needs update label Nov 22, 2019

ghost added the Progress: testing in progress label Nov 22, 2019

ghost added Progress: needs update and removed Progress: testing in progress labels Nov 22, 2019

sidolov changed the base branch from 2.3-develop to 2.4-develop December 5, 2019 17:12

sidolov added Release Line: 2.4 and removed Release Line: 2.3 labels Dec 5, 2019

VladimirZaets closed this Dec 16, 2019

VladimirZaets self-assigned this Dec 16, 2019

All images height issue #25665

All images height issue #25665

Uh oh!

Conversation

abrarpathan19 commented Nov 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

Uh oh!

m2-assistant bot commented Nov 20, 2019

Uh oh!

ihor-sviziev commented Nov 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

m2-assistant bot commented Nov 20, 2019

Uh oh!

abrarpathan19 commented Nov 21, 2019

Uh oh!

ihor-sviziev commented Nov 21, 2019

Uh oh!

abrarpathan19 commented Nov 21, 2019

Uh oh!

krzksz commented Nov 21, 2019

Uh oh!

krzksz Nov 21, 2019

Choose a reason for hiding this comment

Uh oh!

abrarpathan19 Nov 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

engcom-Bravo commented Nov 22, 2019

Uh oh!

krzksz commented Nov 24, 2019

Uh oh!

magento-engcom-team commented Nov 24, 2019

Uh oh!

magento-engcom-team commented Nov 24, 2019

Uh oh!

abrarpathan19 commented Nov 25, 2019

Uh oh!

krzksz commented Nov 25, 2019

Uh oh!

abrarpathan19 commented Nov 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

engcom-Bravo commented Nov 25, 2019

Uh oh!

gwharton commented Nov 25, 2019

Uh oh!

krzksz commented Nov 25, 2019

Uh oh!

magento-engcom-team commented Nov 25, 2019

Uh oh!

magento-engcom-team commented Nov 25, 2019

Uh oh!

krzksz commented Nov 25, 2019

Uh oh!

abrarpathan19 commented Dec 2, 2019

Uh oh!

krzksz commented Dec 2, 2019

Uh oh!

abrarpathan19 commented Dec 2, 2019

Uh oh!

magento-engcom-team commented Dec 2, 2019

Uh oh!

magento-engcom-team commented Dec 2, 2019

Uh oh!

abrarpathan19 commented Dec 2, 2019

Uh oh!

krzksz commented Dec 2, 2019

Uh oh!

VladimirZaets commented Dec 16, 2019

Uh oh!

m2-assistant bot commented Dec 16, 2019

Uh oh!

abrarpathan19 commented Nov 20, 2019 •

edited

Loading

ihor-sviziev commented Nov 20, 2019 •

edited

Loading

abrarpathan19 Nov 22, 2019 •

edited

Loading

abrarpathan19 commented Nov 25, 2019 •

edited

Loading