reactioncommerce / reaction-component-library

Example Storefront Component Library: A set of React components for the Example Storefront
Apache License 2.0
96 stars 57 forks source link

Checkout Action #115

Closed spencern closed 6 years ago

spencern commented 6 years ago

Checkout Action

Overview / Summary

Summary description of UI component

This is a wrapper component that wraps components with the logic for determining whether they are active, complete, or incomplete steps within the checkout process.

Rationale for why this component is necessary

This component is needed to abstract the logic necessary for determining if a checkout action is active, complete, or incomplete.

Expected use cases

This component will be used in the checkout process

Where this component should be created

This component should be created in the component library

Images / Designs of UI component in context

image

image

image

// Active Action
// Pseudo-markup for the Checkout Shipping Address Action
stepNumber activeLabel
<Children />
// COMPLETE Action
// Pseudo-markup for the Checkout Shipping Address Action
<div>
  <CheckoutActionComplete
    completeLabel
    content
    change="handleActionChange"
  />
</div>
// INCOMPLETE Address Action
// Pseudo-markup for the Checkout Shipping Address Action
<div>
  <stepnumber />
  <label />
</div>

Data

Props

propTypes: {
  cart: PropTypes.shape({}),
  isLoading: PropTypes.bool,
  stepNumber: PropTypes.string,
  status: PropTypes.oneOf(["complete", "active", "incomplete"]),
}

Render Criteria

This component will be rendered whenever a shipping address is needed in the checkout process.

UI States

This component is either active, complete, or incomplete https://app.zeplin.io/project/5afc82f5e8d73e250ff9d855/screen/5b02f470eefa9c371dd3c488

Active

In the active state, this component should render the action component associated with it - E.g. if props.action is shippingAddress then this component should render the addressBook. Any state management within the action should be handled by that component.

Complete

In the complete state, this component should render the CheckoutActionComplete component #112

Incomplete

In the incomplete state, this component should render the CheckoutActionIncomplete component

kieckhafer commented 6 years ago

@spencern @aldeed

Do we need more information passed into this component to be able to determine what needs to be rendered?

For instance:

In the active state, this component should render the action component associated with it - E.g. if props.action is shippingAddress then this component should render the addressBook. Any state management within the action should be handled by that component.

Don't we need action passed into this component as well?

And for something like

// COMPLETE Action
// Pseudo-markup for the Checkout Shipping Address Action
<div>
  <CheckoutActionComplete
    completeLabel
    content
    change="handleActionChange"
  />
</div>

We need label, content, and change all available in CheckoutAction, right? It seems like a lot of info needed inside this CheckoutAction component isn't explicitly available. Where is it going to come from?

spencern commented 6 years ago

Yeah, probably. The list of props I gave should really only be a guideline for getting started. Please think deeply about how to tackle this and make suggestions.

kieckhafer commented 6 years ago

I think the best way is to pass a component with it's data into this component as a child, or even as under the components prop.

Although if we do it under the components prop, we'll still need to provide the data here, right? Which might be too much logic / info in this particular new component.

kieckhafer commented 6 years ago

In my attempts to pass in CheckoutActionComplete as a children prop, I'm getting an error TypeError: Cannot read property 'ChangeButton' of undefined, which is from inside CheckoutActionComplete. I'm not 100% sure if this is a legit error, or just an error in the way the docs render the components for preview.

aldeed commented 6 years ago

@kieckhafer Wouldn't that be just because you aren't including components={{ ChangeButton: Button }} on the CheckoutActionComplete element?

aldeed commented 6 years ago

The error would be better if we add components: {} as defaultProps in CheckoutActionComplete, and also add isRequired to the ChangeButton prop type.

kieckhafer commented 6 years ago

@aldeed what are your thoughts on using cloneElement to allow certain props to be passed in only once, and then added to sub-components with cloneElement, as opposed to passing the same props multiple times in different sub-components.

https://github.com/reactioncommerce/reaction-component-library/pull/152/files#diff-e5701e62d2043ba1ed897dcab451c446R23

https://github.com/reactioncommerce/reaction-component-library/pull/152/files#diff-9b732710f13d7ba26016efe3ef4f1dcbR55

label and stepNumber both need to be passed down to ActiveStepComponent, CompleteStepComponent and IncompleteStepComponent.

Doing it how I currently have it in the PR allows us to set that only once, however it takes away the customization if we want to change the label depending on the status.

aldeed commented 6 years ago

@kieckhafer I think that makes sense, but you could do something like if (!CompleteStepComponent.props.label) to check whether label and stepNumber props are already set, and if so, do not overwrite them when cloning.

Also, I'm thinking that we should distinguish between "component" and "element". When we pass in components in other places, it's usually the component class. In this case, it's the rendered component, so "element" or "node" are better terms. Given that you are calling cloneElement with it, probably "element".

Additionally, I think if they are instances rather than classes, the prop names should not be capitalized. So putting those together, maybe rename ActiveStepComponent -> activeStepElement and same for the others.

rc-publisher commented 6 years ago

:tada: This issue has been resolved in version 0.11.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: