Integrating /proposal route and improved proposal editor page #484
Conversation
|
Deploy preview for donut-frontend-r failed. Built with commit 21228dd https://app.netlify.com/sites/donut-frontend-r/deploys/5eff49307d209a0007e3401d |
Rupeshiya
left a comment
Rupeshiya
left a comment
There was a problem hiding this comment.
@AuraOfDivinity If it is WIP PR then it's fine, but if it is going to be merged please remove all the hardcoded data you have used like user token, url etc etc and aslo seperate the request from the component and use it like services or actions.
Also as the redux is already integrated you can use that for managing yout data.
Thank you!
|
@AuraOfDivinity pls fix comments |
Rupeshiya
left a comment
Rupeshiya
left a comment
There was a problem hiding this comment.
@AuraOfDivinity Please go through the comments
src/user/proposals/AdminProposalDashboard/DashboardContent/DashboardContent.js
Outdated
Show resolved
Hide resolved
src/user/proposals/AdminProposalDashboard/DashboardContent/DashboardContent.js
Outdated
Show resolved
Hide resolved
src/user/proposals/AdminProposalDashboard/DashboardContent/DashboardContent.js
Outdated
Show resolved
Hide resolved
src/user/proposals/AdminProposalDashboard/DashboardContent/DashboardContent.js
Outdated
Show resolved
Hide resolved
...user/proposals/ProposalDiscussion/DiscussionContent/DiscussionComments/DiscussionComments.js
Outdated
Show resolved
Hide resolved
src/user/proposals/UserProposalDashboard/DashboardRightPanel/Notifications/Notifications.js
Outdated
Show resolved
Hide resolved
src/user/proposals/UserProposalDashboard/DashboardRightPanel/Notifications/Notifications.js
Outdated
Show resolved
Hide resolved
src/user/proposals/UserProposalDashboard/DashboardRightPanel/Notifications/Notifications.js
Outdated
Show resolved
Hide resolved
src/user/proposals/UserProposalDashboard/DashboardRightPanel/Notifications/Notifications.js
Show resolved
Hide resolved
src/user/proposals/UserProposalDashboard/DashboardRightPanel/Notifications/Notifications.js
Outdated
Show resolved
Hide resolved
src/user/proposals/UserProposalDashboard/DashboardRightPanel/Notifications/Notifications.js
Outdated
Show resolved
Hide resolved
Rupeshiya
left a comment
Rupeshiya
left a comment
There was a problem hiding this comment.
@AuraOfDivinity please check
src/user/proposals/ProposalEditor/EditorContent/EditorContent.js
Outdated
Show resolved
Hide resolved
Rupeshiya
left a comment
Rupeshiya
left a comment
There was a problem hiding this comment.
@AuraOfDivinity Nice work, Please check comments some small changes still required. Also please treat the whole platform as your project. No need to implement everything separately. You can use same instance of the socket for notifications.
...user/proposals/ProposalDiscussion/DiscussionContent/DiscussionComments/DiscussionComments.js
Outdated
Show resolved
Hide resolved
src/user/proposals/UserProposalDashboard/DashboardRightPanel/Utils/socket.js
Outdated
Show resolved
Hide resolved
|
@Rupeshiya @vaibhavdaren @devesh-verma I have made all the requested changes. Please review. |
| if (res.status === 200) { | ||
| dispatch(setRequestStatus(true)) | ||
| console.log('Whole platform notification ', res.data.notifications) | ||
| dispatch(setRequestStatus(true)); |
There was a problem hiding this comment.
are these formatting changes or code changes by linter did you make these?
There was a problem hiding this comment.
@vaibhavdaren These are either due to the extensions he is using or due to his vs code settings.
I have already mentioned that
|
Checks failing |
4 participants
Partially fixes #473
Implementation of the demonstration done on 11/06/20
Improvements to the proposal editor and proposal dashboard pages.
Integrates the usage of backend /proposal routes.
Kindly review @vaibhavdaren @devesh-verma