Skip to content

Supriya : Week2 Assignment#10

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

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

Conversation

@supriyapuri
Copy link
Copy Markdown

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;
Copy link
Copy Markdown

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">
Copy link
Copy Markdown

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" />)}
Copy link
Copy Markdown

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){
Copy link
Copy Markdown

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
Copy Markdown
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?

Copy link
Copy Markdown

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= {
Copy link
Copy Markdown

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()}
Copy link
Copy Markdown

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>
Copy link
Copy Markdown

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