Skip to content

Supriya : Week2 Assignment#10

Open
supriyapuri wants to merge 8 commits intofrontend-application-development-uw20:masterfrom
supriyapuri:master
Open

Supriya : Week2 Assignment#10
supriyapuri wants to merge 8 commits intofrontend-application-development-uw20:masterfrom
supriyapuri:master

Conversation

@supriyapuri
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: 3.5
  • Completion rating on this assignment: complete

@supriyapuri supriyapuri reopened this Apr 22, 2020

export default class AuthorBio extends React.Component{
render(){
const isMediumMember= this.props.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= this.props.isMediumMember;
const { isMediumMember } = this.props;


return(

<div className = "author_details">

Choose a reason for hiding this comment

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

Suggested change
<div className = "author_details">
<div className = "author-details">

Typically html class names are dash-separated


<div className = "author_details">
{isMediumMember? (<img src={this.props.image} alt=" " className= "author_image2" />)
:(<img src={this.props.image} alt=" " className= "author_image1" />)}

Choose a reason for hiding this comment

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

Could this just be something like?

<img src={this.props.image} alt=" " className= "{isMediumMember ? 'author_image1' : 'author_image2' }" />

handleBookmark = (e) =>{
e.preventDefault();

if(!this.state.isBookmarked){

Choose a reason for hiding this comment

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

Nice! typically you'd just want this logic to be in your render function. Don't mess with traversing the document and adding classes, etc.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry I did not quite understand. If i include this inside render function, how should it work without the classes?

Choose a reason for hiding this comment

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

I added a comment to your code below. Basically you want your render function to handle all the DOM manipulation.

constructor(props) {
super(props);

this.state= {

Choose a reason for hiding this comment

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

You wouldn't typically keep static data like this JSON in state.

render(){
return(
<div>
{this.renderForYouSection()}

Choose a reason for hiding this comment

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

I would just put the components here:
<ForYouSection forYouSection={this.state.forYouSection} />

<div className= "additional_details">
<div className = "content_bottom">
<AuthorBio image={this.props.card.author.image} authorName ={this.props.card.author.name} isMediumMember= {isMediumMember} />
<i className="material-icons" id={this.props.id+"-bookmark"} onClick={this.handleBookmark}>bookmark_border</i>

Choose a reason for hiding this comment

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

Suggested change
<i className="material-icons" id={this.props.id+"-bookmark"} onClick={this.handleBookmark}>bookmark_border</i>
<i className="material-icons { this.state.isBookmarked ? : 'bookmark' : '' } " id={this.props.id+"-bookmark"} onClick={this.handleBookmark}>{ this.state.isBookmarked ? : 'bookmark' : 'bookmark_border' }</i>

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