Skip to content

Adds Week 2 homework assignment#1

Open
spencej4 wants to merge 2 commits into
modern-web-application-uw18:masterfrom
spencej4:master
Open

Adds Week 2 homework assignment#1
spencej4 wants to merge 2 commits into
modern-web-application-uw18:masterfrom
spencej4:master

Conversation

@spencej4

Copy link
Copy Markdown

Week 1 HW Submission

Please fill out the information below in order to complete your assignment. Feel free to update this comment later if necessary.

  • Comfort rating on this assignment (1-5):
  • Completion rating on this assignment (complete/incomplete):
  • Github handles of code I've reviewed:

@bhague1281 bhague1281 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you include the rest of the files (package.json, Public folder, etc.) so that we can run this locally? Thank you.

data: PropTypes.objectOf(PropTypes.number),
title: PropTypes.string,
img: PropTypes.string,
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These propTypes are never actually being passed/used by the article components. Instead, I would define propTypes for InnerContent since you're actually passing/utilizing props in that child component.

return (
<div >
{this.state.data.map((title, i) => <InnerContent key={i}
data={title} />)}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent 👌

);
}
}
class InnerContent extends Component {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job defining this component. Since the other InnerContent component is the same, this is an excellent example where you can separate this component into a separate file, then import/use it in both article components.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants