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

Resolve "Clear Shopping Cart" button not working in Internet Explorer issue24491 #24499

Merged
merged 2 commits into from
Sep 12, 2019
Merged

Resolve "Clear Shopping Cart" button not working in Internet Explorer issue24491 #24499

merged 2 commits into from
Sep 12, 2019

Conversation

edenduong
Copy link
Contributor

Description (*)

Resolve #24491: "Clear Shopping Cart" button not working in Internet Explorer

The problem:

in this PR: #21499 "Cart is emptied when enter is pressed after changing product quantity". This code is added to app/code/Magento/Checkout/view/frontend/web/js/shopping-cart.js

                if (event.detail === 0) {
                    return;
                }

However, IE 11 can not support event.detail. It always return 0

https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/16710370/

I believe when enter is pressed after changing product quantity we Shouldn't trigger click "emptyCartButton" and check the condition. I will be wrong. We should change the button empty cart to "button", not "submit". So button isn't clicked when press enter.

In my PR, I have changed it to normal button and process the empty cart only the user click to the "Clear shopping cart" button.

Fixed Issues (if relevant)

  1. "Clear Shopping Cart" button not working in Internet Explorer #24491: "Clear Shopping Cart" button not working in Internet Explorer

Manual testing scenarios (*)

Change Magento Theme to BLANK

  1. Open Magento 2 site in Internet Explorer
  2. Add item to the cart
  3. Click Clear Shopping Cart button

Expected result (*)

  1. All items are deleted from the shopping cart
  2. The cart page is refreshed showing no items

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)

@m2-assistant
Copy link

m2-assistant bot commented Sep 7, 2019

Hi @edenduong. 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.

@dmytro-ch dmytro-ch self-assigned this Sep 7, 2019
@dmytro-ch dmytro-ch self-requested a review September 7, 2019 12:31
@dmytro-ch dmytro-ch added the Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests label Sep 7, 2019
@magento-engcom-team
Copy link
Contributor

Hi @dmytro-ch, thank you for the review.
ENGCOM-5807 has been created to process this Pull Request

@dmytro-ch
Copy link
Contributor

@magento give me 2.3-develop instance

@magento-engcom-team
Copy link
Contributor

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

@magento-engcom-team
Copy link
Contributor

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

@ghost ghost assigned VladimirZaets Sep 9, 2019
@magento-engcom-team
Copy link
Contributor

Hi @VladimirZaets, thank you for the review.
ENGCOM-5807 has been created to process this Pull Request

DrewML
DrewML reviewed Sep 9, 2019
View reviewed changes
app/code/Magento/Checkout/view/frontend/web/js/shopping-cart.js
$(this.options.emptyCartButton).attr('name', 'update_cart_action_temp');
$(this.options.updateCartActionContainer)
.attr('name', 'update_cart_action').attr('value', 'empty_cart');

if ($(this.options.emptyCartButton).parents('form').length > 0) {
Copy link

Choose a reason for hiding this comment

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

Can you describe why this fixes the reported bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DrewML: I remove event.detail == 0 because it isn't supported by IE.11.

event.detail code is added to fix the bug #21499.

I removed it and changed to other solution to fix issue 21499. When the cart is updated, it should't call the emptyCartButton. So I change it to "button" element. So the emptyCartButton isn't trigger click when the qty is update.

@engcom-Alfa engcom-Alfa self-assigned this Sep 11, 2019
@engcom-Alfa
Copy link
Contributor

@magento give me test instance

@magento-engcom-team
Copy link
Contributor

Hi @engcom-Alfa. Thank you for your request. I'm working on Magento instance for you

@magento-engcom-team
Copy link
Contributor

Hi @engcom-Alfa, here is your new Magento instance.
Admin access: https://pr-24499.instances.magento-community.engineering/admin
Login: admin Password: 123123q

@engcom-Alfa
Copy link
Contributor

@magento give me 2.3-develop instance

@magento-engcom-team
Copy link
Contributor

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

@magento-engcom-team
Copy link
Contributor

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

@engcom-Alfa
Copy link
Contributor

✔️ QA Passed

@engcom-Foxtrot engcom-Foxtrot self-assigned this Sep 12, 2019
This was referenced Sep 12, 2019
Closed
Closed
magento-engcom-team pushed a commit that referenced this pull request Sep 12, 2019
@sidolov
ENGCOM-5807: Resolve "Clear Shopping Cart" button not working in Inte…
619d02a 
…rnet Explorer issue24491 #24499
@magento-engcom-team magento-engcom-team merged commit 5847d9f into magento:2.3-develop Sep 12, 2019
@m2-assistant
Copy link

m2-assistant bot commented Sep 12, 2019

Hi @edenduong, 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 magento-engcom-team added this to the Release: 2.3.4 milestone Sep 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DrewML DrewML DrewML left review comments

@VladimirZaets VladimirZaets VladimirZaets approved these changes

@dmytro-ch dmytro-ch dmytro-ch approved these changes

@paliarush paliarush Awaiting requested review from paliarush

Assignees

@VladimirZaets VladimirZaets

@dmytro-ch dmytro-ch

@engcom-Alfa engcom-Alfa

@engcom-Foxtrot engcom-Foxtrot

Labels
Area: Frontend Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests Award: bug fix Component: Checkout Progress: accept Release Line: 2.3
Projects
None yet
Milestone
Release: 2.3.4
Development

Successfully merging this pull request may close these issues.

"Clear Shopping Cart" button not working in Internet Explorer
7 participants
@edenduong @magento-engcom-team @dmytro-ch @engcom-Alfa @DrewML @VladimirZaets @engcom-Foxtrot