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.

Sign up for GitHub

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

Jump to bottom

#164798170 User Should be able to comment on articles #35

Merged
merged 1 commit into from
May 22, 2019
Merged

#164798170 User Should be able to comment on articles #35

merged 1 commit into from
May 22, 2019

Conversation

ezrogha
Copy link
Contributor

@ezrogha ezrogha commented May 20, 2019 •
edited
Loading

What does this PR do?

Allow authenticated users to comment on articles

Description of Task to be completed?

  • Allow user to view all comments
  • Allow user to comment on articles
  • Allow user to reply to a comment
  • Allow user to edit and delete a comment
  • Allow user to edit and delete a reply

How should this be manually tested?

  • Fetch this branch git fetch origin ft-comment-article-164798170
  • Checkout to the branch git checkout ft-comment-article-164798170
  • Run npm start to view the landing page
  • Login to the app on /Login
  • Choose article to comment on on dashboard

What are the relevant pivotal tracker stories?

#164798170

Screenshots (if appropriate)

Screenshot 2019-05-20 at 10 03 35

felixkiryowa
felixkiryowa reviewed May 20, 2019
View reviewed changes
src/actions/commentActions.js Outdated
location.reload();
})
.catch((error) => {
console.log(error);
Copy link
Contributor

Choose a reason for hiding this comment

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

@ezrogha remove the console.log`
Because we agreed that, we should not be leaving them anywhere in our code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for that feedback,
@felixkiryowa

src/components/Articles/ArticleDetail.jsx Outdated
@@ -79,6 +80,7 @@ const ArticleDetail = ({ article }) => (
</button>
</div>
<DeleteArticleComponentModel />
<Comments comments={comments} showReplies={showReplies} replyDisplayState={replyDisplayState} slug={article.slug} postReplyToComment={postReplyToComment} />
Copy link
Contributor

Choose a reason for hiding this comment

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

@ezrogha
Can you reduce on the length of this line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @felixkiryowa,
Feedback implemented

src/components/Comment/Comment.scss
.commentDate {
font-size: 12px;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@ezrogha change the px to rem

Copy link
Contributor Author

@ezrogha ezrogha May 22, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the feedback @felixkiryowa,
It has been implemented

dannylwe
dannylwe previously requested changes May 20, 2019
View reviewed changes
src/components/ReplyToComment/ReplyToComment.scss
border-radius: 2px;
}

.commentReplyButtn {
Copy link
Contributor

Choose a reason for hiding this comment

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

@ezrogha change px to rem

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback @dannylwe,
It has been implemented

src/pages/Comments/index.jsx Outdated

const Comments = ({ comments, showReplies, replyDisplayState, slug, postReplyToComment }) => (
<div>
<div style={{ marginTop: 70, marginBottom: 15 }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

@ezrogha put these in css file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for that feedback @dannylwe,
It has been implemented

@ezrogha ezrogha force-pushed the ft-comment-article-164798170 branch 5 times, most recently from 87b3ae9 to f349f72 Compare May 22, 2019 10:01
@ezrogha
ft(comment): User should comment on article:
ea6b2f4 
  - fetch all comments
  - fetch all replies to comment
  - post comment
  - post reply to comment

[Maintains #164798170]
@ezrogha ezrogha force-pushed the ft-comment-article-164798170 branch from f349f72 to ea6b2f4 Compare May 22, 2019 10:07
hadijahkyampeire
hadijahkyampeire approved these changes May 22, 2019
View reviewed changes
Copy link

@hadijahkyampeire hadijahkyampeire left a comment

Choose a reason for hiding this comment

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

looks great but next time avoid change irrelevant files

@hadijahkyampeire hadijahkyampeire merged commit 8a6bcac into develop May 22, 2019
@ezrogha ezrogha added Finished Label for when a task is done and removed in progress labels May 22, 2019
dorothyas pushed a commit that referenced this pull request May 23, 2019
@ezrogha @dorothyas
ft(comment): User should comment on article: (#35)
465c4e5 
- fetch all comments
  - fetch all replies to comment
  - post comment
  - post reply to comment

[Maintains #164798170]
dorothyas pushed a commit that referenced this pull request May 23, 2019
@ezrogha @dorothyas
ft(comment): User should comment on article: (#35)
1fcb6cf 
- fetch all comments
  - fetch all replies to comment
  - post comment
  - post reply to comment

[Maintains #164798170]
dorothyas pushed a commit that referenced this pull request May 23, 2019
@ezrogha @dorothyas
ft(comment): User should comment on article: (#35)
b8a8757 
- fetch all comments
  - fetch all replies to comment
  - post comment
  - post reply to comment

[Maintains #164798170]
dorothyas pushed a commit that referenced this pull request May 23, 2019
@ezrogha @dorothyas
ft(comment): User should comment on article: (#35)
a9ac207 
- fetch all comments
  - fetch all replies to comment
  - post comment
  - post reply to comment

[Maintains #164798170]
dorothyas pushed a commit that referenced this pull request May 23, 2019
@ezrogha @dorothyas
ft(comment): User should comment on article: (#35)
bc7fd38 
- fetch all comments
  - fetch all replies to comment
  - post comment
  - post reply to comment

[Maintains #164798170]
dorothyas pushed a commit that referenced this pull request May 23, 2019
@ezrogha @dorothyas
ft(comment): User should comment on article: (#35)
f4deee0 
- fetch all comments
  - fetch all replies to comment
  - post comment
  - post reply to comment

[Maintains #164798170]
dorothyas pushed a commit that referenced this pull request May 23, 2019
@ezrogha @dorothyas
ft(comment): User should comment on article: (#35)
d5823bb 
- fetch all comments
  - fetch all replies to comment
  - post comment
  - post reply to comment

[Maintains #164798170]
dorothyas pushed a commit that referenced this pull request May 23, 2019
@ezrogha @dorothyas
ft(comment): User should comment on article: (#35)
d081c63 
- fetch all comments
  - fetch all replies to comment
  - post comment
  - post reply to comment

[Maintains #164798170]
dorothyas pushed a commit that referenced this pull request May 23, 2019
@ezrogha @dorothyas
ft(comment): User should comment on article: (#35)
9bbc854 
- fetch all comments
  - fetch all replies to comment
  - post comment
  - post reply to comment

[Maintains #164798170]
hadijahkyampeire pushed a commit that referenced this pull request May 23, 2019
@dorothyas @hadijahkyampeire
ft(comment): User should comment on article: (#35) (#40)
f53f3c8 
- fetch all comments
  - fetch all replies to comment
  - post comment
  - post reply to comment

[Maintains #164798170]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hadijahkyampeire hadijahkyampeire hadijahkyampeire approved these changes

@dannylwe dannylwe Awaiting requested review from dannylwe

@felixkiryowa felixkiryowa Awaiting requested review from felixkiryowa

@dorothyas dorothyas Awaiting requested review from dorothyas

Assignees
No one assigned
Labels
Finished Label for when a task is done
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ezrogha @dannylwe @felixkiryowa @hadijahkyampeire