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.

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

Career page #783

Merged
merged 10 commits into from
Sep 18, 2020
Merged

Career page #783

merged 10 commits into from
Sep 18, 2020

Conversation

vinayaksh42
Copy link
Member

@vinayaksh42 vinayaksh42 commented Sep 17, 2020

Signed-off-by: Vinayak Sharma vinayaksh42@gmail.com

Description
Migration of careers page from previous layer5 website to layer5-ng
previouse website ---> layer5-ng
This PR fixes #710

Notes for Reviewers
I have made some ui changes while migrating from the previous layer5 Website to the current layer5-ng website according to my own thought process. So kindly guide on that and suggest any changes if needed

Signed commits

  • Yes, I signed my commits.

Signed-off-by: Vinayak Sharma <vinayaksh42@gmail.com>
@netlify
Copy link

netlify bot commented Sep 17, 2020

Deploy preview for layer5ng ready!

Built with commit af05a97

https://deploy-preview-783--layer5ng.netlify.app

Signed-off-by: Vinayak Sharma <vinayaksh42@gmail.com>
Signed-off-by: Vinayak Sharma <vinayaksh42@gmail.com>
Signed-off-by: Vinayak Sharma <vinayaksh42@gmail.com>
Signed-off-by: Vinayak Sharma <vinayaksh42@gmail.com>
@vinayaksh42
Copy link
Member Author

@Tanuj22 @Nikhil-Ladha can you review my pr?

Copy link
Contributor

@Nikhil-Ladha Nikhil-Ladha 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. But few improvements can be done:

  • The What is Layer5 section, can be moved above the video.
  • In the opportunities section, all the internship cards can be moved below the internship subheading and a new Full-Time Opportunities subheading can be created and the other cards can be put below it.
  • The design of the cards for the programs can be improved a bit, like the border can be made a bit more prominent.
  • Also, See our careers page... line can be removed as we are already on the careers page

Also, @leecalcote any text or info that can be added for Full time opportunities like internship ?

Signed-off-by: Vinayak Sharma <vinayaksh42@gmail.com>
Signed-off-by: Vinayak Sharma <vinayaksh42@gmail.com>
@vinayaksh42
Copy link
Member Author

@Nikhil-Ladha ,let me know if there is any content to be added under the full time sections. For the time being i have just added it without any description

Signed-off-by: Vinayak Sharma <vinayaksh42@gmail.com>
@Nikhil-Ladha
Copy link
Contributor

Okay, so for the time being do one thing move the text part below the internship section, above it below the Available Opportunities heading.

  • Move the subheading Full time opportunities a bit down, more towards the below displayed cards rathwr than the top.
    Screenshot_20200918-094616__01

  • The bottom and top borders of the cards in the middle can be made more prominent.
    Also, on hovering the card in mobile view the zoom effect is good, but it leads the card to overlap with the other cards.
    Screenshot_20200918-094631__01

Signed-off-by: Vinayak Sharma <vinayaksh42@gmail.com>
@vinayaksh42
Copy link
Member Author

Made the changes you requested @Nikhil-Ladha . Tho i am not able to understand what you mean by making the borders more prominent.

Signed-off-by: Vinayak Sharma <vinayaksh42@gmail.com>
Copy link
Contributor

@Nikhil-Ladha Nikhil-Ladha 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 now.
Nice work 🚀 @vinayaksh42

@Nikhil-Ladha Nikhil-Ladha merged commit 3a86181 into layer5io:layer5-ng Sep 18, 2020
@s-pratyush s-pratyush mentioned this pull request Sep 25, 2020
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants