Popup card for Shipping

[rvennam]

No description provided.

[coveralls]

Coverage remained the same at 96.739% when pulling cea2097 on PopUpCard into b1228f4 on dev.

[colbycheeze]

best practice to use rem for spacing.
Actually, you generally want to stay away from em unless you have a very good / specific reason to use it...such as some sort of font based thing.

[coveralls]

Coverage remained the same at 96.739% when pulling 004a341 on PopUpCard into b1228f4 on dev.

[colbycheeze]

We can refactor this later if you want, but it's probably more helpful to assume nothing about the API giving the data in the component, and ask for props exactly as it would be helpful to render them.

For example, you may want to ask for an object like:

shipment: {
  location: 'City, State',
  lastUpdated: 'time',
  eta: 'time',
  orderId: 123,
  status: 'pending',  
}

and then, also we want to do a full prop validation for the object coming in. Here is how I did it for the role switcher:

RoleItem.propTypes = {
  user: React.PropTypes.shape({
    id: React.PropTypes.number.isRequired,
    role: React.PropTypes.string.isRequired,
    location: React.PropTypes.string,
    loggedIn: React.PropTypes.bool,
  }),
  roleAction: React.PropTypes.func.isRequired,
};

[rvennam]

That's how I had it before, but then changed it like this because it was adding a lot of parsing logic to the parent component (Dashboard)....

[colbycheeze]

yea that's fine, it's why I mentioned we can refactor later. Ideally the wrapper component, or the reducer holding the state would be doing all of this logic, which doesn't exist yet.

[colbycheeze]

The main idea, or way of thinking is to keep all business logic outside of the view components of the app, such that as the app grows and multiple pieces may need to consume that data it's not being reproduced everywhere.

The other benefit, is that if...say we wanted to build a React Native app later...that React Native app would easily be able to consume all of our reducers and other logic...and all they had to do was build some new views that are specific to Ios or android etc.

[colbycheeze]

Another thing you might add, is a PopUpCard.test.js file that for now just imports your component so that coveralls picks it up. You don't have to write the tests right now I can show you the minimal tests we should do for the UI components (mostly just checking that props do what we expect, and functions are called when expected)

[coveralls]

Coverage increased (+0.3%) to 97.059% when pulling a9b21f7 on PopUpCard into b1228f4 on dev.

[colbycheeze]

Okay I'm ready to merge this. Another thing that might help in future pull requests is to toss in a screenshot and/or gif of what you added if it's a visual component. Not a big deal, just easier to review.