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

task solution was added #926

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

Conversation

Liliya-Kalinichenko
Copy link

@Liliya-Kalinichenko Liliya-Kalinichenko commented Sep 28, 2023

Copy link

@maxim2310 maxim2310 left a comment

Choose a reason for hiding this comment

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

Great job overall! Just couple tiny things needs to be fixed before approval)

  1. when after error i have correct request i still have error message
    image
  2. and the same after bad request for comment i cant reach good scenario
  3. need to add email validation
  4. after adding new comment need to clear inputs

Copy link

@anastasiia-tilikina anastasiia-tilikina left a comment

Choose a reason for hiding this comment

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

  1. pay attention to this requirement:
    image

  2. when something went wrong with adding comment I can't try again

Screen.Recording.2023-09-29.at.10.56.41.mov

Comment on lines 184 to 185
setBody(event.target.value);
setIsBodyError(false);

Choose a reason for hiding this comment

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

it is bad practise to make anonymous arrow functions in jsx.
make handler in component and pass reference here

Misha Hrynko
</a>
return (
<>

Choose a reason for hiding this comment

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

don't see any sense to use <> here, because there is only one element inside

getComments(post.id)
.then(setComments)
.catch(() => {
setIsCommentLoading(false);

Choose a reason for hiding this comment

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

you already have setIsCommentLoading in finally block

Comment on lines 77 to 79
handleOpenPostClick(post);
setIsCommentLoadError(false);
setIsCommentDeleteError(false);

Choose a reason for hiding this comment

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

it is bad practise to make anonymous arrow functions in jsx.
make handler in component and pass reference here

Copy link

@lerastarynets lerastarynets left a comment

Choose a reason for hiding this comment

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

You've got this, keep pushing forward!

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.

5 participants