Thanks. Some more nitpiks and test improvements we could do:

+++ b/modules/d...

Contribution Date
Technology
Contribution Project
Contribution Details

Thanks. Some more nitpiks and test improvements we could do:

  1. +++ b/modules/dc_ajax_add_cart_popup/README.md @@ -0,0 +1,10 @@ +The is a submodule of Commerce Ajax Add to Cart. This displays a modal popup after

    s/The is a sumodule/A submodule.

  2. +++ b/modules/dc_ajax_add_cart_popup/src/EventSubscriber/AjaxAddToCartPopupSubscriber.php @@ -0,0 +1,101 @@ + if (!$this->purchasedEntity) {

    We should write a test for this.

  3. +++ b/modules/dc_ajax_add_cart_popup/src/EventSubscriber/AjaxAddToCartPopupSubscriber.php @@ -0,0 +1,101 @@ + if (!$response instanceof AjaxResponse) {

    We should write a test for this.

  4. +++ b/modules/dc_ajax_add_cart_popup/tests/src/FunctionalJavascript/AjaxAddCartPopupTest.php @@ -0,0 +1,59 @@ + // Confirm that the initial add to cart submit works. + $this->cart = Order::load($this->cart->id()); + $order_items = $this->cart->getItems(); + $this->assertOrderItemInOrder($this->variation, $order_items[0]);

    Will not be required. We are already testing in the parent module's test.

  5. +++ b/modules/dc_ajax_add_cart_popup/tests/src/FunctionalJavascript/AjaxAddCartPopupTest.php @@ -0,0 +1,59 @@ + $this->assertSession()->pageTextContains("The item has been added to your cart.", 'Popup not found.');

    This is okay. But it would be great, if we test this backend-wise as well. Like, check if the current response contains OpenModalDialogCommand.

  6. +++ b/modules/dc_ajax_add_cart_popup/tests/src/FunctionalJavascript/AjaxAddCartPopupTest.php @@ -0,0 +1,59 @@ + $this->assertSession()->pageTextNotContains("The item has been added to your cart.", 'Popup was found before AJAX finished.');

    This is okay. But it would be great, if we test this backend-wise as well. Like, check if the current response contains OpenModalDialogCommand.

Thanks for the effort Brock. I was looking into the bugs that were reported few days ago.

The patch looks perfect. I think we can use this idea in your patch to fix #2930926: Cart block should be updated (ajax) when item removed from cart. I will try to do the fixes next week, and do a manual check on my local. Thank you.

Contribution Author
Files count
0
Patches count
0