Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Prevent product gallery elements from blinking when they load #27871

Merged
merged 7 commits into from
Jun 5, 2020
Merged

Prevent product gallery elements from blinking when they load #27871

merged 7 commits into from
Jun 5, 2020

Conversation

krzksz
Copy link
Contributor

@krzksz krzksz commented Apr 16, 2020 •
edited by ghost
Loading

Description (*)

In this PR I would like to improve the loading experience of the gallery on product page. On the attached screenshot you can see that there are small periods of time when:

  1. Product image disappears and there is a white space until gallery initializes and its images load.
  2. Left arrow is shown and then hidden after gallery loads.

Before:

Gallery loading before changes

After:

Gallery loading after changes
Note: There is a loader shown properly for a few frames, it just didn't get captured on the gif.

Manual testing scenarios (*)

  1. Gallery placeholder image is positioned properly (as before) on all supported browsers and resolutions.
  2. Gallery works as before when initialized.

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)

Resolved issues:

  1. resolves [Issue] Prevent product gallery elements from blinking when they load #28339: Prevent product gallery elements from blinking when they load

@m2-assistant
Copy link

m2-assistant bot commented Apr 16, 2020

Hi @krzksz. 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.4-develop instance - deploy vanilla Magento instance

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

@ghost ghost added this to Pending Review in Pull Requests Dashboard Apr 16, 2020
@ihor-sviziev ihor-sviziev self-assigned this Apr 16, 2020
@ihor-sviziev ihor-sviziev removed the request for review from ptylek April 16, 2020 08:52
ihor-sviziev
ihor-sviziev requested changes Apr 16, 2020
View reviewed changes
Copy link
Contributor

@ihor-sviziev ihor-sviziev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @krzksz,
Your changes looks good to me!
Could you review unit & static test failures and update your PR?

@ghost ghost moved this from Pending Review to Changes Requested in Pull Requests Dashboard Apr 16, 2020
@krzksz
Copy link
Contributor Author

krzksz commented Apr 18, 2020

All tests green 💚

@ghost ghost moved this from Changes Requested to Ready for Testing in Pull Requests Dashboard Apr 18, 2020
@ghost ghost assigned ptylek Apr 18, 2020
@magento-engcom-team
Copy link
Contributor

Hi @ptylek, thank you for the review.
ENGCOM-7456 has been created to process this Pull Request
✳️ @ptylek, could you please add one of the following labels to the Pull Request?

Label Description
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests

@ptylek
Copy link
Contributor

ptylek commented Apr 18, 2020 •
edited
Loading

@krzksz do you think this change could have some additional tests? it seems to be improvement rather than gallery functionality change and all current tests have passed.

@krzksz
Copy link
Contributor Author

krzksz commented Apr 18, 2020

I think it is impossible to cover this change with anything automatic. There are already some unit tests which failed previously and I had to adjust them so the implementation is untouched.

@ptylek ptylek added the Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests label Apr 18, 2020
@slavvka slavvka added Priority: P3 May be fixed according to the position in the backlog. Severity: S3 Affects non-critical data or functionality and does not force users to employ a workaround. labels Apr 22, 2020
@slavvka slavvka added this to the 2.4.1 milestone Apr 22, 2020
@VladimirZaets VladimirZaets added the QA: Ready to add to Regression Scope Should be analyzed and added to Regression Testing Scope(if applicable) label May 19, 2020
@VladimirZaets VladimirZaets moved this from Merge in Progress to Extended Testing (optional) in Pull Requests Dashboard May 19, 2020
@sdzhepa
Copy link
Contributor

sdzhepa commented May 22, 2020

@magento create issue

@ghost ghost mentioned this pull request May 22, 2020
Closed
@sdzhepa sdzhepa linked an issue May 22, 2020 that may be closed by this pull request
[Issue] Prevent product gallery elements from blinking when they load #28339
Closed
@sdzhepa
Copy link
Contributor

sdzhepa commented May 22, 2020

@magento run all tests

@sdzhepa sdzhepa added QA: Added to Regression Scope Scenario was analysed and added to Regression Testing Scope and removed QA: Ready to add to Regression Scope Should be analyzed and added to Regression Testing Scope(if applicable) labels May 22, 2020
@engcom-Echo
Copy link
Contributor

@magento run all tests

@engcom-Echo
Copy link
Contributor

Failed functional tests not related to the changes in this PR

@engcom-Echo engcom-Echo moved this from Extended Testing (optional) to Merge in Progress in Pull Requests Dashboard May 27, 2020
@slavvka
Copy link
Member

slavvka commented May 27, 2020

@magento run all tests

magento-engcom-team pushed a commit that referenced this pull request Jun 5, 2020
@slavvka
ENGCOM-7456: Prevent product gallery elements from blinking when they…
fb7afcd 
… load #27871
@magento-engcom-team magento-engcom-team merged commit 65ac6ff into magento:2.4-develop Jun 5, 2020
@m2-assistant
Copy link

m2-assistant bot commented Jun 5, 2020

Hi @krzksz, 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.

@ghost ghost moved this from Merge in Progress to Recently Merged in Pull Requests Dashboard Jun 5, 2020
@ihor-sviziev ihor-sviziev mentioned this pull request Mar 1, 2021
Closed
5 tasks
@tpaaro tpaaro mentioned this pull request Dec 21, 2021
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ptylek ptylek ptylek approved these changes

@ihor-sviziev ihor-sviziev Awaiting requested review from ihor-sviziev

Assignees

@ihor-sviziev ihor-sviziev

@ptylek ptylek

@engcom-Alfa engcom-Alfa

@engcom-Echo engcom-Echo

Labels
Area: Lib/Frontend Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests Award: bug fix Partner: creativestyle partners-contribution Pull Request is created by Magento Partner Priority: P3 May be fixed according to the position in the backlog. Progress: accept QA: Added to Regression Scope Scenario was analysed and added to Regression Testing Scope Release Line: 2.4 Severity: S3 Affects non-critical data or functionality and does not force users to employ a workaround.
Projects
Pull Requests Dashboard
  
Recently Merged
Milestone
2.4.1
Development

Successfully merging this pull request may close these issues.

[Issue] Prevent product gallery elements from blinking when they load
9 participants
@krzksz @magento-engcom-team @ptylek @engcom-Alfa @sdzhepa @engcom-Echo @slavvka @ihor-sviziev @VladimirZaets