Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sammy: project-happy-thoughts #436

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

sammyolsson
Copy link

No description provided.

Copy link

@Nahnahke Nahnahke left a comment

Choose a reason for hiding this comment

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

I would like to start out by letting you know that you should be proud of this weeks work! You did a really good job and I'm super impressed by your structure.

There really isn't much negative to say about your code except for the word break in the single-thought container. That is all I would like to add.

Big high five on a great job with a tough assignment!

setNewThought(event.target.value)
}

/* POSTS NEW THOUGHT AND UPDATES STATE */

Choose a reason for hiding this comment

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

Good job commenting!

</div>
);
}
}

Choose a reason for hiding this comment

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

Very nice structure in your App.js! Impressive!

Comment on lines +6 to +7
<p>Happy Thoughts made by <a href="https://www.linkedin.com/in/sammy-olsson/" target="_blank" rel="noreferrer">Sammy Olsson</a>,
Team-🐢

Choose a reason for hiding this comment

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

Nice add with the contact information!

Comment on lines +4 to +5
const ThoughtForm = ({ newThought, onNewThoughtChange, onFormSubmit }) => {
const isSubmitButtonDisabled = newThought.length < 6 || newThought.length > 140;

Choose a reason for hiding this comment

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

Short and consistent code on the length parameters.

<section className="form-section">
<form className="form" onSubmit={onFormSubmit}>
<label className="label" htmlFor="thought-input">
What&apos;s making you happy right now?

Choose a reason for hiding this comment

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

Good job adding the: '

import React from 'react';
import { formatDistanceToNow } from 'date-fns';

const ThoughtList = ({ loading, thoughtList, handleLike }) => {

Choose a reason for hiding this comment

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

Very clear and structured code and props. Your code is easy to follow and understand.

Comment on lines +26 to +27
<div className="time-wrapper">
<span className="time">{formatDistanceToNow(

Choose a reason for hiding this comment

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

Smart solution with the newDate.

}

.thought-input {
font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", "Roboto", "Oxygen",

Choose a reason for hiding this comment

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

Do you need all these fonts? Perhaps make a --var in the beginning so you won't have to write as much code for each.

margin: 1.5rem 1rem 1rem 1rem;
box-shadow: 0.5rem 0.5rem rgb(31, 31, 31);
padding: 2rem;
}

Choose a reason for hiding this comment

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

I suggest you try adding overflow-wrap: break-word; to your .single-though to break the words in the container. Now they are over flowing.

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