Skip to content

Sven Otto - week 2 submission#1

Open
Bl4ckH4wkGER wants to merge 3 commits intofrontend-application-development-uw20:masterfrom
Bl4ckH4wkGER:master
Open

Sven Otto - week 2 submission#1
Bl4ckH4wkGER wants to merge 3 commits intofrontend-application-development-uw20:masterfrom
Bl4ckH4wkGER:master

Conversation

@Bl4ckH4wkGER
Copy link

Week 2 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: 2
    I felt like I was still missing the following crucial basic knowledge going into this exercise:
  • how to properly define and structure components
  • how to pull json data into react components
  • using propTypes

Over the course of the week I spent 20h on this assignment, a lot of which was reading documentation. I was hoping for more reference material and clarifying examples from the slides and inclass code.

  • Completion rating on this assignment: complete
    I believe it is complete. PropTypes and CSS may we a little wonky

}

function AuthorBio(props) {
const isMediumMember = props.source.isMediumMember;

Choose a reason for hiding this comment

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

Suggested change
const isMediumMember = props.source.isMediumMember;
const { isMediumMember } = props.source;

className='missed-article author-image'
style={ isMediumMember ? {border: '5px solid green'} : {}}
src={props.source.image}
alt={props.source.image}

Choose a reason for hiding this comment

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

Typically the alt tag would be human readable, not a URL

}

class MissedArticles extends React.Component {
static propTypes = {

Choose a reason for hiding this comment

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

Nice work here

)
}

function DateAndDuration(props) {

Choose a reason for hiding this comment

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

I personally find these nested render functions within the same file somewhat hard to parse/understand. I guess it's personal preference, but I typically like to look in one place for the render function in a file to determine what the DOM output is going to look like.

)
}

function AuthorBio(props) {

Choose a reason for hiding this comment

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

Is this the same AuthBio as in missed-articles.js? Consider refactoring and importing to each.

@t1mwillis
Copy link

Really great work!

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