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] Hover Effect on Social Media Icons. #64

Merged
merged 1 commit into from
Apr 14, 2018

Conversation

utkarsh2102
Copy link
Contributor

@utkarsh2102 utkarsh2102 commented Apr 13, 2018

This PR is made in reference to the issue #63.

I've just added a hovering effect on social media icons on -

  • index.html
  • coc.html
  • sponsorship.html
  • faq.html

Hover Effect :

  • opacity:0.5;
  • z-index:10

Site preview available at : https://utkarsh2102.github.io/inpycon2018/

Copy link
Member

@realslimshanky realslimshanky left a comment

Choose a reason for hiding this comment

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

Changes LGTM. You've also refactored some of the code. I don't think what I feel about this. Since even after you fixed some code there's a lot of code left which can be fixed with similar operation.

@utkarsh2102
Copy link
Contributor Author

utkarsh2102 commented Apr 13, 2018

@realslimshanky, I did not do it manually.
It's just a default setting in my text editor to remove all the extra spacing present in the page.
If you want, I can undo this and add my code only.
However, I don't think I have altered any code, it's just the spaces which were removed.
Let me know if it's still a problem?

@realslimshanky
Copy link
Member

realslimshanky commented Apr 13, 2018

It's okay. @ananyo2012 @chirag200666 would you like to take a look at this PR?

@ananyo2012
Copy link
Contributor

@utkarsh2102 Looks good. Let me test this out in my local.

@ananyo2012
Copy link
Contributor

Ok you added the preview. Just checking it.

Copy link
Contributor

@ananyo2012 ananyo2012 left a comment

Choose a reason for hiding this comment

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

LGTM. Merging this.

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.

3 participants