-
Notifications
You must be signed in to change notification settings - Fork 77
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
fix: Fix profile edit form on submission #119
Conversation
@codesankalp @keshakaneria @Aaishpra kindly review it |
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! Replace the green color with a bit of dark shade and black text written in ALL CAPS with added exclamation mark at the end for Success, so that it is visible. Same goes for the red shade for unsuccessful.
You can use the following shades:
- Green: #4BE043
- Red: #F61E1E
@keshakaneria sure will implement those any other suggestions except for that. |
Otherwise looks good 👍 |
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.
See my review comments @ruddi10.
@@ -17,6 +17,7 @@ | |||
"react-router-dom": "^5.2.0", | |||
"react-router-redux": "^4.0.8", | |||
"react-scripts": "3.4.1", | |||
"react-toastify": "^7.0.3", |
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.
I am not sure about adding this, but it looks good. @Aaishpra and @keshakaneria What do you think about adding this?
(I am asking this because it is mentioned to use semantic-UI.)
@@ -1,80 +1,74 @@ | |||
import axios from 'axios' | |||
import axios from "axios"; |
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.
Why this file has been changed @ruddi10 as there is no need for change in actions.
If it's just re-formatting then unstage this file.
@@ -1,22 +1,19 @@ | |||
import React, { Component } from 'react' |
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.
Again, If there is no change added by you in the code of this file then unstage that file because there is a PR for this.
@@ -1,62 +1,69 @@ | |||
import React, { Component } from 'react' |
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.
Also, unstage this file if there is no change made by you.
@ruddi10 Please resolve the merge conflicts and do the suggested changes. |
Closing this PR due to inactivity and no response from @ruddi10 |
@codesankalp I will complete this by this weekend if that's not a problem? |
Ok, @ruddi10 reopening this pull request 👍 |
@ruddi10 Any updates? |
@codesankalp some serious health issues in family will it be fine if I take some time? |
Ok, @ruddi10 take the time but keep updating in a few days. |
@ruddi10 Can you please update your progress? I will wait for 2 days for your reply and after that I will close this PR. |
@ruddi10 I will close this due to lack of response. I hope you and your family are well now 🙏🏾 cc @codesankalp |
Description
When a user edit or fill the profile even after pressing the submit button the form remains open and I think that creates confusion I would rather suggest that we close the form .
New Dependency
react-toastify
Fixes #117
Type of Change:
Code/Quality Assurance Only
How Has This Been Tested?
Checklist:
Code/Quality Assurance Only