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

Resolved issue #13, #14 and #15 #31

Closed
wants to merge 4 commits into from
Closed

Resolved issue #13, #14 and #15 #31

wants to merge 4 commits into from

Conversation

Mahak008
Copy link
Contributor

@Mahak008 Mahak008 commented Dec 31, 2023 •
edited
Loading

You can watch the demo here -
https://mahak008.github.io/PortfolioCreations/

image

@Mahak008 Mahak008 changed the title Experience Section Added Experience Section Added - Resolved issue #14 Dec 31, 2023
@Mahak008 Mahak008 changed the title Experience Section Added - Resolved issue #14 Experience Section and Download Resume button Added - Resolved issue #13 and #14 Jan 1, 2024
@Mahak008 Mahak008 changed the title Experience Section and Download Resume button Added - Resolved issue #13 and #14 Resolved issue #13, #14 and #15 Jan 1, 2024
@ayushichoudhary-19
Copy link
Member

@Mahak008, thank you for your PR. I've noticed a few points for improvement:

  1. The colors used don't align with the website's color palette. Please ensure consistency with the established color scheme.

  2. Consider relocating the "Download CV" button. Having it fixed on the screen at all times might affect the overall layout. Perhaps, placing it in the navbar or header section could be a better fit.

  3. Regarding the loader, it doesn't quite align with the design aesthetics, and the loading timespan is too short for a thorough check. Let's adjust it to match the overall design and ensure a more user-friendly loading experience.

Looking forward to your updates. Thanks!

Nitya-Pasrija
Nitya-Pasrija reviewed Jan 1, 2024
View reviewed changes
Copy link
Contributor

@Nitya-Pasrija Nitya-Pasrija left a comment

Choose a reason for hiding this comment

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

Furthermore, I strongly suggest creating 3 different PRs to resolve the 3 issues, instead of one single PR. Do look into branching. Feel free to contact us if you are confused :)

@Nitya-Pasrija
Copy link
Contributor

Also, there are conflicting files as well. Please look into it.

@Mahak008
Copy link
Contributor Author

Mahak008 commented Jan 2, 2024

@Mahak008, thank you for your PR. I've noticed a few points for improvement:

  1. The colors used don't align with the website's color palette. Please ensure consistency with the established color scheme.
  2. Consider relocating the "Download CV" button. Having it fixed on the screen at all times might affect the overall layout. Perhaps, placing it in the navbar or header section could be a better fit.
  3. Regarding the loader, it doesn't quite align with the design aesthetics, and the loading timespan is too short for a thorough check. Let's adjust it to match the overall design and ensure a more user-friendly loading experience.

Looking forward to your updates. Thanks!

Ok I will make the required changes. Can you please close this PR. Iwill create separate PR for all the issues.

@ayushichoudhary-19
Copy link
Member

Sure, @Mahak008! Feel free to make the necessary changes. I'll go ahead and close this pull request for now. Looking forward to seeing those separate pull requests for each issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nitya-Pasrija Nitya-Pasrija Nitya-Pasrija left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Mahak008 @ayushichoudhary-19 @Nitya-Pasrija