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.

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

JQuery FlexSlider: Fix innerHeight computation #33847

Merged
merged 3 commits into from
Aug 3, 2022

Conversation

gigitux
Copy link
Contributor

@gigitux gigitux commented Jul 13, 2022

All Submissions:

Changes proposed in this Pull Request:

Issue #33829 is caused by the innerHeight computation that is done by the library flexslider The computation depends on the dimension of different slides. The problem is that the computation of the height is done BEFORE setting the new dimensions to the slides. This PR fixes this behavior.

⚠️ ESLint

I removed a console.log that I guess is not necessary. Also, ESLint gives me a lot of errors, but I didn't apply the fix, otherwise, the changes would be very massive.

Closes #33829 .

How to test the changes in this Pull Request:

  1. Install Storefront.
  2. Create a Product and add multiple images to the product.
  3. Go to the Product Page.
  4. Open the device simulation.
  5. Set iPhone 12 Pro and rotate the device orientation.
  6. Be sure that the first image of the product, is rendered correctly, and if you click on the page, no element changes height.

I suggest checking the attached video to understand the problem.

178453842-7b2e2f41-b2ca-4239-ac85-24ff6eb93a25.mp4

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?
  • Have you created a changelog file for each project being changed, ie pnpm changelog add --filter=<project>?

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@gigitux gigitux self-assigned this Jul 13, 2022
@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Jul 13, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jul 13, 2022

Test Results Summary

Commit SHA: 3533522

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests11500201170m 37s
E2E Tests185001018613m 36s
To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

@ObliviousHarmony ObliviousHarmony requested review from a team and barryhughes and removed request for a team July 13, 2022 16:06
@barryhughes
Copy link
Member

Re-running failed tests (looks like some were temporary glitches, timeouts etc).

@barryhughes
Copy link
Member

@gigitux could you add a changelog entry?

pnpm changelog add --filter=plugins/woocommerce

@gigitux
Copy link
Contributor Author

gigitux commented Aug 1, 2022

@gigitux could you add a changelog entry?

pnpm changelog add --filter=plugins/woocommerce

Hi!
Thanks for the suggestion. I added the changelog!

@barryhughes barryhughes merged commit 397a784 into trunk Aug 3, 2022
@barryhughes barryhughes deleted the fix/33829-flexslider-calculation branch August 3, 2022 16:20
@github-actions github-actions bot added this to the 6.9.0 milestone Aug 3, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2022

Hi @barryhughes, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

  • Add the release: add testing instructions label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: woocommerce Issues related to the WooCommerce Core plugin.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Storefront single product with multiple images resizing issue
2 participants