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

Added social icons on the left of the website and changed the title o… #183

Merged
merged 6 commits into from
Jun 6, 2021

Conversation

RiyaGupta89
Copy link
Contributor

@RiyaGupta89 RiyaGupta89 commented Jun 2, 2021

…f the home page

Description

Please include a summary of the change and which issue is fixed. List any dependencies that are required for this change.

Fixed # (issue) (e.g. Fixed #8)

Type of change

Please check options that are relevant to your PR.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Checklist:

  • I have squashed my commits
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

@RiyaGupta89
Copy link
Contributor Author

I have added social icons on the left of the website as it was mentioned in the issues section of the project. Please check it out. I hope you find it relevant.

Copy link
Member

@Manvityagi Manvityagi left a comment

Choose a reason for hiding this comment

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

On smaller screens, the icons overlap with the text in main area.
Check the ideal behavior here: https://happy-shockley-488e36.netlify.app/
The exact same thing has been implemented here - Girl-Code-It/Opportunity-Calendar-Frontend#250

Also, GitHub icon is missing.

Thanks for the efforts and PR, appreciated :)

@RiyaGupta89
Copy link
Contributor Author

RiyaGupta89 commented Jun 2, 2021 via email

@Manvityagi
Copy link
Member

Do you want me to make it mobile friendly? I can do that.

On Wed, 2 Jun, 2021, 6:16 pm Manvi Tyagi, @.***> wrote: @Manvityagi requested changes on this pull request. On smaller screens, the icons overlap with the text in main area. Check the ideal behavior here: https://happy-shockley-488e36.netlify.app/ The exact same thing has been implemented here - Girl-Code-It/Opportunity-Calendar-Frontend#250 <Girl-Code-It/Opportunity-Calendar-Frontend#250> Also, GitHub icon is missing. Thanks for the efforts and PR, appreciated :) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#183 (review)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARIR5XPAZYQXRXWAVHCMQ2DTQYRZPANCNFSM456HS6LA .

Yes, exactly

@RiyaGupta89
Copy link
Contributor Author

Made it mobile friendly. Do you want me to do something with the navbar links? Like hover effect or add active class on it?

@Manvityagi
Copy link
Member

Made it mobile friendly. Do you want me to do something with the navbar links? Like hover effect or add active class on it?

Made it mobile friendly. Do you want me to do something with the navbar links? Like hover effect or add active class on it?

You have made that part perfect!!
Here are a couple of thoughts:

  1. It is fixed on bottom of screen, I liked it better on mid-screen like this
  2. Infact, The layout itself , the size, color and positioning - I feel this way again suits more.

CC: @vaishali614 What do you think?

@RiyaGupta89
Copy link
Contributor Author

Fixed the links to the mid of the page and also changed the layout.

@Manvityagi
Copy link
Member

Fixed the links to the mid of the page and also changed the layout.

Awesome, One last change - The headline should be "From Will to Skill", There is also an issue #177 for the same.

@RiyaGupta89
Copy link
Contributor Author

Yes. Done.

Copy link
Member

@Manvityagi Manvityagi left a comment

Choose a reason for hiding this comment

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

Other than little changes in the comments,

tag42cee592abc7d668.gif
See this GIF, From around 763w to 663, the blue line is above the text.

If you see the original website, this transition is smooth and consistent.
tagOrig.gif

<Container className={styles.container}>
<Row>
<Col md={6} lg={7}>
<h1 className={styles.heading}>
Give wings to <br className={styles.align} /> your
Embrace <br className={styles.align} /> your
Copy link
Member

Choose a reason for hiding this comment

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

From Will to Skill here too

<Container className={styles.containersmall} fluid>
<h1 style={{ textAlign: "center" }} className={styles.heading}>
Give wings to <br className={styles.align} /> your
<span style={{ color: "#008dc8" }}> dreams </span>
From Will to <br className={styles.align} /> to
Copy link
Member

Choose a reason for hiding this comment

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

From Will to to Skill.

To is two times here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have a look now.

Copy link
Member

Choose a reason for hiding this comment

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

The layout "From Will to Skill" needs to be revised, because the text is small, it isn't looking good on the banner.

Let's do one thing, Revert all your changes in the banner, (Keep it the original one - Give wings to your dreams), and just give social media icons in this PR. I will merge right away.

Copy link
Member

@Manvityagi Manvityagi left a comment

Choose a reason for hiding this comment

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

Awesome Work @RiyaGupta08. Thanks for the PR. Merging this

@Manvityagi Manvityagi merged commit b6fd4e0 into Girl-Code-It:develop Jun 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make CodeMapCard and CodeMap Component
3 participants