-
Notifications
You must be signed in to change notification settings - Fork 317
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
feat: show contactus data on admin dashboard #829
feat: show contactus data on admin dashboard #829
Conversation
Thank you and congrats🎉 for opening this pull request!💖 Please make sure you have followed our Contributing Guidelines.🙌 We will test out your code and reply in a bit with some pointers and requests.💿 There may be some errors, but don't worry! We'll work through them with you! 👍🎉😄 If you haven't filled the template out, Please do so!🚀 |
@iMADi-ARCH Please explain changes you made in detail. |
@thevirengarg The logic and ui is the same as of the joinus page. |
Also the Reply button has a mailto link so as to facilitate replying to the messages. |
@iMADi-ARCH , Can you please keep the colors used consistent and not include multiple colors? Specifically, Red color seems to go off. Rest is great. |
Are these good? sorry I am not very good at choosing colors 😓 |
Okay so for now you can replace the red color with the pink tone used throughout the website. |
Yeah, good for now 👍🏻 |
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.
@iMADi-ARCH First, You need to get the issue assigned to you and then make a PR.
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.
Go to the issue and comment on it so I can assign it you and then I will review this PR.
#740 has no more assignment tag |
Just ignore it, comment on it then i will assign it to you |
i did just now. |
Please assign him the issue and process the review as per repository workflow of accepting PRs. |
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.
improve the styling of the cards which should resonate with the website's theme and also add some comments in your code. It is advisable to add screenshots of before and after in your PR description while making improvements to the UI .
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.
Styling issues to be addressed before merging.
Every page in dashboard has almost the same card styling which is what i have followed. Also there should have been a common card component and the button component is also being exported as Button1, Button2, Button3 etc which does not makes much sense. Maybe an issue should be opened to refactor some of the styling. I will add comments to the code. |
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.
@iMADi-ARCH Everything is fine, just add some comments.
Congrats on merging your first pull request! 🙌 🎉 ⚡️ Now that you've merged your first pull request, you're the perfect person to help someone else out with this challenging first step.😄🙌 |
Issue that this pull request solves
Closes: #740
Proposed changes
Display the contactus data as fetched from
/getContactUs
endpoint.Brief description of what is fixed or changed
Types of changes
Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that applyScreenshots
Please attach the screenshots of the changes made in case of change in user interface
Other information
**Note: ** delete functionality has not been implemented on the backend as of creating this PR and thus this doesn't include the same.