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

Bug:improved labels,placeholder update user page #450

Merged
merged 2 commits into from
Oct 13, 2022
Merged

Bug:improved labels,placeholder update user page #450

merged 2 commits into from
Oct 13, 2022

Conversation

tanishk-23
Copy link
Contributor

Fixes Issue

My PR Closes #380

Changes proposed

What were you told to do ?

I was told to add labels and placeholders which were missing in update user page on small screen devices.

What did you do ?

I added placeholders which were missing in update user page and also changed display property of labels to font-size: large

Check List (Check all the applicable boxes)

  • My code follows the code style of this project.
  • This PR does not contain plagiarized content.
  • The title and description of the PR is clear and explains the approach.

Screenshots

Mobile View
MobileView

Desktop View
DesktopView

@vercel
Copy link

vercel bot commented Oct 10, 2022 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
milan ✅ Ready (Inspect) Visit Preview Oct 13, 2022 at 5:43AM (UTC)

github-actions[bot]
github-actions bot reviewed Oct 10, 2022
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hello contributor, thank you for opening a Pull Request 🎉.

Soon the maintainers/owner will review it and provide you with feedback/suggestions.
If you think it's something urgent, feel free to reach out Tamal Das on Twitter.
Star ⭐ this repo to show us support.

Happy, Open Source!

@vercel vercel bot temporarily deployed to Preview October 10, 2022 08:14 Inactive
tamalCodes
tamalCodes requested changes Oct 10, 2022
View reviewed changes
Copy link
Member

@tamalCodes tamalCodes left a comment

Choose a reason for hiding this comment

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

  • Make it conditional such that placeholders are seen only for mobiles

Example : https://milaan.vercel.app/user/login
image

@tamalCodes tamalCodes added area/logic This issue is related to a logic fix. 🟧 priority: high area/ui This issue is related to frontend/interface fix. 🐞 bug This is a patch, generally fixing some bugs. status: need changes 🚧 Changes have been requested for these PRs status: wip 👩🏻‍💻 This issue/PR is actively being worked on. labels Oct 10, 2022
@tanishk-23
Copy link
Contributor Author

@iamtamal I have done the suggested changes. Please review and let me know if i can make PR.

screencapture-localhost-3000-user-profile-2022-10-10-21_07_08

@vercel vercel bot temporarily deployed to Preview October 10, 2022 20:43 Inactive
@vercel vercel bot temporarily deployed to Preview October 11, 2022 05:45 Inactive
@tamalCodes
Copy link
Member

tamalCodes commented Oct 11, 2022 •
edited
Loading

@tanishk-23 Push the changes to the PR please ! It looks good from the screenshot !

@vercel vercel bot temporarily deployed to Preview October 11, 2022 06:18 Inactive
@tanishk-23
Copy link
Contributor Author

@iamtamal I have pushed the changes to the PR.

@vercel vercel bot temporarily deployed to Preview October 11, 2022 06:55 Inactive
@tanishk-23
Copy link
Contributor Author

@iamtamal If you are satisfied with the changes then please merge my PR and if further changes are required please let me know.

@vercel vercel bot temporarily deployed to Preview October 12, 2022 10:00 Inactive
@tamalCodes
Copy link
Member

@iamtamal If you are satisfied with the changes then please merge my PR and if further changes are required please let me know.

image

Placeholders should not be visible on desktop.

@tanishk-23
Copy link
Contributor Author

@iamtamal Implemented the changes.

UpdateChange

@tamalCodes
Copy link
Member

@tanishk-23 You don't need to tag me and post SS everytime. Simply push the code to PR, we can review easily that way !

@vercel vercel bot temporarily deployed to Preview October 12, 2022 15:41 Inactive
@vercel vercel bot temporarily deployed to Preview October 13, 2022 05:43 Inactive
tamalCodes
tamalCodes approved these changes Oct 13, 2022
View reviewed changes
Copy link
Member

@tamalCodes tamalCodes left a comment

Choose a reason for hiding this comment

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

Looks good to me ✅

  • ⚒ Thankyou for working on the issue
  • ⭐ Do star the repo, to get updates on more issues !
  • 💚 Keep contributing to Opensource

@tamalCodes tamalCodes added status: ready ✅ This PR has passed all the checks and is now ready to be merged. hacktoberfest-accepted This PR is accepted for Hacktoberfest. and removed status: need changes 🚧 Changes have been requested for these PRs status: wip 👩🏻‍💻 This issue/PR is actively being worked on. labels Oct 13, 2022
@tamalCodes tamalCodes merged commit 9a66235 into ngoworldcommunity:main Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions GitHub Actions github-actions left review comments

@tamalCodes tamalCodes tamalCodes approved these changes

Assignees

@tanishk-23 tanishk-23

Labels
area/logic This issue is related to a logic fix. area/ui This issue is related to frontend/interface fix. 🐞 bug This is a patch, generally fixing some bugs. hacktoberfest-accepted This PR is accepted for Hacktoberfest. status: ready ✅ This PR has passed all the checks and is now ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bug: labels and placeholder missing in update user page
2 participants
@tanishk-23 @tamalCodes