Skip to content

Conversation

@Den4ik
Copy link
Contributor

@Den4ik Den4ik commented Jul 24, 2019

Description (*)

This PR provide complete fix for wrong behaviour of collapsible library on product page and category page.
Previous fix was broke category page filter behaviour

Fixed Issues (if relevant)

  1. Changed logic so that _scrollToTopIfVisible is called only if element is in viewport. Previously it was called only when the element was outside it. #23390

Manual testing scenarios (*)

  1. Go to category page
  2. Try to click onto filter headers
  3. Expected that the page will not make strange jumps

Questions or comments

Expected result

r06vh_2019-07-24_16-27-23
pp81n_2019-07-24_16-24-59

Actual result

9o16q_2019-07-24_16-32-42

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)

@m2-assistant
Copy link

m2-assistant bot commented Jul 24, 2019

Hi @Den4ik. 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 Assistant documentation

@Den4ik Den4ik force-pushed the bug/wrong-collapse-behaviour branch from cd5880a to 263537f Compare July 25, 2019 07:17
@Den4ik Den4ik requested review from orlangur and sidolov July 26, 2019 13:32
@Den4ik
Copy link
Contributor Author

Den4ik commented Aug 9, 2019

@sidolov could you check this PR. Previous fix was broke catalog filters collapse

@VladimirZaets VladimirZaets self-assigned this Aug 12, 2019
@magento-engcom-team
Copy link
Contributor

Hi @VladimirZaets, thank you for the review.
ENGCOM-5600 has been created to process this Pull Request
✳️ @VladimirZaets, 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

@engcom-Alfa
Copy link
Contributor

✔️ QA Passed

Before:
before

After:
after

@m2-assistant
Copy link

m2-assistant bot commented Aug 15, 2019

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

magento-engcom-team pushed a commit that referenced this pull request Aug 15, 2019
@magento-engcom-team magento-engcom-team added this to the Release: 2.3.4 milestone Aug 15, 2019
@Karlasa
Copy link
Contributor

Karlasa commented Aug 28, 2019

@magento give me test instance

@magento-engcom-team
Copy link
Contributor

Hi @Karlasa. Thank you for your request. I'm working on Magento instance for you

@Karlasa
Copy link
Contributor

Karlasa commented Aug 28, 2019

@magento give me 2.3-develop instance

@magento-engcom-team
Copy link
Contributor

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

@magento-engcom-team
Copy link
Contributor

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

@Den4ik Den4ik deleted the bug/wrong-collapse-behaviour branch August 28, 2019 10:18
@VladimirZaets VladimirZaets added the Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests label Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants