daffodil icon indicating copy to clipboard operation
daffodil copied to clipboard

[FEAT] Reduce Code Complexity of Demo's Cart Module

Open damienwebdev opened this issue 6 years ago • 1 comments

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[x] Feature request
[ ] Documentation issue or request
[ ] Other... Please describe:

Current behavior

The demo's cart module has a two extremely complex components:

  1. cart-wrapper
  2. checkout-cart-async-wrapper

that are really hard to understand. They interact with store and also have @Input's.

Expected behavior

There are a few more small "dumb" components" and a potentially a few more "smart" components to lower the overall complexity of the module.

What is the motivation / use case for changing the behavior?

Maintainability and code legibility.

damienwebdev avatar Nov 28 '18 15:11 damienwebdev

So it seems like the first step is to extract some of the markup of cart-wrapper into separate components. Should header and sidebar be separate components as well or do you not want components that thin?

<div class="cart-wrapper">
  <div class="cart-wrapper__main">
    <div class="cart-wrapper__header">
      <span class="cart-wrapper__title">Your Cart</span>

      <!-- cart-wrapper-item-count -->
      <span class="cart-wrapper__item-count" [ngPlural]="itemCount$ | async">
        {{(itemCount$ | async)}}
        <ng-template ngPluralCase="other">Items</ng-template>
        <ng-template ngPluralCase="=1">Item</ng-template>
      </span>
      <!-- /cart-wrapper-item-count -->
      
    </div>
    <demo-cart [cart]="cart"></demo-cart>
  </div>
  <div class="cart-wrapper__sidebar">

    <!-- cart-wrapper-summary -->
    <div class="cart-wrapper__summary" *ngIf="!(isCartEmpty$ | async)">
      <div class="cart-wrapper__summary-title">Cart Summary</div>
      <demo-cart-totals class="cart-wrapper__cart-totals" [cart]="cart"></demo-cart-totals>
      <button type="button" daff-button color="secondary" class="cart-wrapper__button" demoProceedToCheckout>
        Proceed to Checkout
      </button>
    </div>
    <!-- /cart-wrapper-summary -->

    <demo-help-box class="cart-wrapper__help-box"></demo-help-box>
  </div>
</div>

Then the internal state of cart-wrapper

They interact with store and also have @Input's.

I suppose you prefer that data is passed from a parent component into the child that uses it to make that child easier to test? If so, cart-wrapper-item-count would have itemCount passed in from cart-wrapper. As for isCartEmpty, I assume you would want it passed in from the cart-view page component?

I couldn't find a component named checkout-cart-async-wrapper, was it removed?

Anything else that you want cleaned up in this module?

griest024 avatar Jan 02 '20 00:01 griest024