Some observations (mostly nitpiks):

+++ b/modules/dc_ajax_add_cart_popup/REA...

Some observations (mostly nitpiks):

1. +++ b/modules/dc_ajax_add_cart_popup/README.md @@ -0,0 +1,10 @@ +- Go to `admin/commerce/config/product-variation-types/default/edit/display`, check + **Ajax add to cart popup** under `Custom Display Settings`, and click `Save`.

   Could you please add this instructions in the parent module's README. This should be added under Installation and usage of the parent README.

2. +++ b/modules/dc_ajax_add_cart_popup/dc_ajax_add_cart_popup.info.yml @@ -0,0 +1,7 @@ + - dc_ajax_add_cart

   s/dc_ajax_add_cart/drupal:dc_ajax_add_cart. Sometimes coder throws error if not done like that.

3. +++ b/modules/dc_ajax_add_cart_popup/dc_ajax_add_cart_popup.module @@ -0,0 +1,34 @@ + 'product_variation_label' => NULL,

   Not required. We are passing the whole entity.

4. +++ b/modules/dc_ajax_add_cart_popup/dc_ajax_add_cart_popup.module @@ -0,0 +1,34 @@ +function dc_ajax_add_cart_popup_form_alter(&$form, &$form_state, $form_id) {

   I have a feeling that we can replace hook_form_alter() with a more specific hook_form_BASE_FORM_ID_alter (). And remove the inner if condition.

5. +++ b/modules/dc_ajax_add_cart_popup/src/EventSubscriber/AjaxAddToCartPopupSubscriber.php @@ -0,0 +1,82 @@ + $view_builder = \Drupal::entityManager()->getViewBuilder('commerce_product_variation');

   Use depedency injection instead.

6. +++ b/modules/dc_ajax_add_cart_popup/src/EventSubscriber/AjaxAddToCartPopupSubscriber.php @@ -0,0 +1,82 @@ + '#product_variation_label' => $this->purchasedEntity->label(),

   We don't need this. We are already passing the full entity.

7. +++ b/modules/dc_ajax_add_cart_popup/src/EventSubscriber/AjaxAddToCartPopupSubscriber.php @@ -0,0 +1,82 @@ + $title = '';

   Remove this. This is not used anywhere.

8. +++ b/modules/dc_ajax_add_cart_popup/src/EventSubscriber/AjaxAddToCartPopupSubscriber.php @@ -0,0 +1,82 @@ + $options = ['width' => '700'];

   Ideally this should be coming from an admin setting. But not to worry now. We can do that later, in a separate issue.

9. +++ b/modules/dc_ajax_add_cart_popup/tests/src/FunctionalJavascript/AjaxAddCartPopupTest.php @@ -0,0 +1,57 @@ + 'commerce_product', + 'commerce_cart', + 'dc_ajax_add_cart', + 'dc_ajax_add_cart_popup',

   Just do dc_ajax_add_cart_popup. Others will be enabled as dependent modules.

10. +++ b/modules/dc_ajax_add_cart_popup/tests/src/FunctionalJavascript/AjaxAddCartPopupTest.php @@ -0,0 +1,57 @@ + protected $profile = 'standard';

    Have you checked running this in the default testing profile.

11. +++ b/modules/dc_ajax_add_cart_popup/tests/src/FunctionalJavascript/AjaxAddCartPopupTest.php @@ -0,0 +1,57 @@ + public function testPopup() {

    AWESOME!! AWESOME!! This is great. Thank you for adding the tests without being asked.

    Sorry for my last comment. I just skimmed the patch last time, didn't checked it fully.

12. +++ b/modules/dc_ajax_add_cart_popup/tests/src/FunctionalJavascript/AjaxAddCartPopupTest.php @@ -0,0 +1,57 @@ + $this->waitForAjaxToFinish();

    I have also added negative tests. Add another test that would check whether the popup is indeed ajaxified. I check that by not calling $this->waitForAjaxToFinish().

I would say that your work is almost perfect :) ? Thank you Brock.