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/highlight new comments #897
Feature/highlight new comments #897
Conversation
Fixes #732 |
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 are a few things I noticed so far while testing:
- Click to a proposal and leave the route by clicking "back" in the browser, the componentWillUnmount isn't called, thus the access time isn't logged. Maybe move it to didMount or right after getting the last accesstime?
- Every new comment I make after opening a route is marked as new. The comments written by the current user shouldn't be highlighted. The motivation of this feature is to help the user to find comments he may not have seen yet.
Also, every time I make a new comment it request the accesstime route, It doesn't look correct.
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.
The code looks fine, I just have an inline comment.
src/components/snew/ThingComment.js
Outdated
@@ -74,6 +74,7 @@ class ThingComment extends React.PureComponent { | |||
...props | |||
} = this.props; | |||
const isProposalAbandoned = proposalStatus === PROPOSAL_STATUS_ABANDONED; | |||
console.log(lastVisit); |
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.
You left a log here.
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.
Fixed!
- add request handle when logging out - fix proposal access time handler on react lifecycle - remove SubmitPage log - remove componentWillMount - route refactor - remove console logs - change requests according to new structure on politeia - remove logs upgrade snew-classic-ui on yarn.lock add user access time handler according to the new schema
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.
This happens because the accesstime is set only when the proposal is fetched. This was discussed on Slack and I was told that this was the best way to deal with the situation, since I was not allowed to set the accesstime on a new request. |
LGTM |
No description provided.