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

Add spacing above the heading #5653

Closed
wants to merge 3 commits into from
Closed

Conversation

r-kundan
Copy link
Contributor

@r-kundan r-kundan commented Jul 8, 2024

Description

This PR fixes #5648

Notes for Reviewers

Signed commits

  • Yes, I signed my commits.

@l5io
Copy link
Contributor

l5io commented Jul 8, 2024

🚀 Preview for commit 1919cfa at: https://668c20c5d681547e79ca73ef--layer5.netlify.app

@Muhammed-Moinuddin
Copy link
Contributor

@r-kundan there should be some spacing in the bottom of heading as previously suggested. You missed that. Kindly follow this page: https://layer5.io/projects

@l5io
Copy link
Contributor

l5io commented Jul 9, 2024

🚀 Preview for commit b038cd8 at: https://668cdaea27123c42136a5348--layer5.netlify.app

Copy link
Contributor

@Ashparshp Ashparshp left a comment

Choose a reason for hiding this comment

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

Hi @r-kundan,

LGTM!! Thank you for making the changes. Apologies for the oversight on the issue assignments. I've noticed your consistent interest in working on various issues. Unfortunately, assignments are made in chronological order, which sometimes results in issues already being assigned. We greatly appreciate your participation and contributions to the project.

Thanks again!

Copy link
Contributor

@Ashparshp Ashparshp left a comment

Choose a reason for hiding this comment

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

Also, please add some space above the top heading. Do not add space between "Open Source Internship Programs" and "Build Your Career at Layer5"—definitely not that much.

@vishalvivekm vishalvivekm requested a review from Ashparshp July 14, 2024 18:22
@vishalvivekm
Copy link
Member

@r-kundan
Thank you for your contribution.
Let's discuss this on Websites' call. Add this as an agenda item into the meeting minutes, if you would :)

@l5io
Copy link
Contributor

l5io commented Jul 15, 2024

🚀 Preview for commit 6d1e38b at: https://66950145c84a200092beda48--layer5.netlify.app

@l5io
Copy link
Contributor

l5io commented Jul 15, 2024

🚀 Preview for commit c263b90 at: https://669511fdc84a2015a8beda10--layer5.netlify.app

Copy link
Contributor

@Muhammed-Moinuddin Muhammed-Moinuddin left a comment

Choose a reason for hiding this comment

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

@r-kundan changes in file i.e. (src/reusecore/PageHeader/pageHeader.style.js) is also impacting another page i.e. (https://layer5.io/projects).
So, we need to make changes in a way that both pages look good. Let's discuss via slack.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @r-kundan,

Screenshot from 2024-07-10 22-41-16

I checked your code, it's completely fine but we can achieve a bit of code consistency if instead of directly adding spacing we remove h3 from h1 div and do a liitle of css tweaks. if you closely via inspect on projects page (https://layer5.io/projects) you'll find that.

That way we'll achieve both solution and code consistency.

@leecalcote
Copy link
Member

Keep at it, @r-kundan. Almost there...

@r-kundan r-kundan closed this Jul 19, 2024
@r-kundan r-kundan deleted the space-added branch July 19, 2024 16:31
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.

Add spacing above the heading
6 participants