-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feature form #7
Feature form #7
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Status: Required Changes ♻️
Hi @IsmailChangezi,
Good job so far!
There are some issues that you still need to work on to go to the next project but you are almost there!
Highlights
- Implemented some project requirements. ✔️
- No linter errors. ✔️
- Good README file. ✔️
- Correct Gitflow. ✔️
Required Changes ♻️
Check the comments under the review.
Optional suggestions
Every comment with the [OPTIONAL] prefix is not crucial enough to stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better.
Cheers and Happy coding!👏👏👏
Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me in your question so I can receive the notification.
Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.
As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.
<!DOCTYPE html> | ||
<html lang="en"> | ||
<head> | ||
<meta charset="UTF-8"> | ||
<meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
<title>New Post</title> | ||
</head> | ||
<body> | ||
<p>Current user: <%= @user.name %> </p> | ||
<h1>Create New Post</h1> | ||
<div class="container"> | ||
<%= form_with(model: @post, url: user_posts_path, method: :post) do |form| %> | ||
<div class="form-container"> | ||
<div class="flex form-title"> | ||
<%= form.label :title, 'Title: ' %> | ||
<%= form.text_field :title %> | ||
</div> | ||
<%= form.text_area :text, placeholder: "Enter content of the article here" %> | ||
<%= form.hidden_field :author_id, value: @user.id %> | ||
|
||
<%= form.submit 'Create Post' %> | ||
</div> | ||
<% end %> | ||
</div> | ||
|
||
</body> | ||
</html> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Great job adding form to your app. However, there is no place on the page where this form is displayed which will allow the current user to create posts. I looked around but could not find a form to create posts on the page. Kindly look into that and fix.
User page.
your-page.webm
<!-- Creating error message displaying the data that failed --> | ||
<% if @comment.errors.any? %> | ||
<div id="error_explanation"> | ||
<h2><%= pluralize(@comment.errors.count, "error") %> prohibited this comment from being saved:</h2> | ||
|
||
<ul> | ||
<% @comment.errors.full_messages.each do |msg| %> | ||
<li><%= msg %></li> | ||
<% end %> | ||
</ul> | ||
</div> | ||
<% end %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- It is required to have a form where the current user can add comments to a post. That is not implemented yet. Kindly add that too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved 🙌
Good job for implementing the changes suggested by the previous reviewer 💪 Your project is COMPLETE! There is nothing else to say other than... it's time to merge it! 🚀 Congratulations!
Highlights 🎯
There are some excellent points that you have made perfectly. Keep up the great work! 👏
- ✅ Form for adding is post is present
- ✅ Great work using the right GitFlow
- ✅ Your README looks good
- ✅ Linters are working properly
Optional suggestions ❕
Please have a look at the comment below
Every comment with the [OPTIONAL] prefix is not crucial enough to stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better.
Cheers and Happy coding!👏👨💻💻
Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me in your question so I can receive the notification.
Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.
As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.
<% if @comment.errors.any? %> | ||
<div id="error_explanation"> | ||
<h2><%= pluralize(@comment.errors.count, "error") %> prohibited this comment from being saved:</h2> | ||
|
||
<ul> | ||
<% @comment.errors.full_messages.each do |msg| %> | ||
<li><%= msg %></li> | ||
<% end %> | ||
</ul> | ||
</div> | ||
<% end %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
[OPTIONAL] You have done a great job for creating a form to add comments ✨💪, However, I think it would be better to have a form where the current user can add comments to a post in
views/comments/new.html.erb
file which accepts input for creating comments. I noticed that you have a form inviews/posts/show.html.erb
However, it is violating the principles of separation of concerns and modularity in web development 📌⛏ Each component of your application (models, views, controllers) should have a clear and distinct responsibility.- The
posts/show.html.erb
view is meant for displaying information about a single post, not for creating new posts/comments. - The
comments/new.html.erb
view, on the other hand, should be responsible for rendering the form to create a new comment.
Mixing these responsibilities can make the codebase less maintainable and harder to understand✨ So, please make sure to keep the comment creation form in its own dedicated view (
views/comments/new.html.erb
) - The
Description