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

Enhance the mailing option and updated the icons #233

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

nitinjaiswal7537
Copy link

Enhanced the mailing option and have updated the mail and calling icons

@garg3133
Copy link
Owner

@nitinjaiswal7537 Seems like you didn't test anything. The email is not centred w.r.t the icon, click on email takes us to some non-existing web-page instead of opening an email client, hover effect on email doesn't work, the logo on contact us side banner looks really odd (there was absolutely no need to change that...

For phone icon, you can use fas fa-phone fa-rotate-90.

@nitinjaiswal7537
Copy link
Author

Hey @garg3133 sorry for the mistake but the hovering option is not showing its result despite using different alternative there my be some technical issue by my side and also the alignment task.

@garg3133
Copy link
Owner

@nitinjaiswal7537 The hover effect is not working because hover-effect of a is already white and instead of applying the new hover effect on the a tag (by putting the .email class in a), you've applied the new hover effect on the wrapping p tag. As properties on a is more specific than p (a is written inside p), properties on a will take precedence. So, you may try applying the new hover effect on the a instead of p.

For centring the email w.r.t. icon, I had shared some tips with you in the very first PR, if I remember correctly. You may try those out.

@nitinjaiswal7537
Copy link
Author

Hey @garg3133 here are the few alternatives i had tried
a .email:hover{
background-color: orange;
}
div.con-box1 > a.email:hover {
background-color:orange;
}
but still it not reflecting anything just only one thing that on tapping its shows orange color that all
and nor did the alignment part goes as per desired result So what should i do ??

@garg3133
Copy link
Owner

garg3133 commented Apr 17, 2021

Where have you put the .email class? If you've put it in p, none of the above-mentioned alternatives would work.

@nitinjaiswal7537
Copy link
Author

nitinjaiswal7537 commented Apr 18, 2021

No no i have assign a tag with the email class.

  <div class="con-box1" data-aos="fade-down-left">
                <p ><a class="email" href="mailto:jagrati@iiitdmj.ac.in" style="display: inline-block; text-align: justify;"><i class="fas fa-envelope fa-2x"></i> <span>jagrati@iiitdmj.ac.in</span></a>
                
        </div>

Have a look over this code.

@garg3133
Copy link
Owner

With this code also, both the alternatives you suggested won't work.

  1. It should be written as a.email:hover (no space between a and .email as both belong to the same element.
  2. Omit > from here. > refers to the child element only and not the grandchild. I'd recommend this one after omitting >.

@nitinjaiswal7537
Copy link
Author

Hey @garg3133 i have tried the above alternative it still not reflecting any changes.

@garg3133
Copy link
Owner

Well, I used the following HTML:

<div class="con-box1" data-aos="fade-down-left">
  <p>
    <a href="jagrati@iiitdmj.ac.in" class="contact-us-email">
      <i class="fa fa-envelope-o fa-2x"></i>
       jagrati@iiitdmj.ac.in
    </a>
  </p>
</div>

and the following CSS:

.con-box1 .contact-us-email:hover{
	color: #ff8c00;
}

and it's working fine for me.

@nitinjaiswal7537
Copy link
Author

@garg3133 I think there should surely ne some kind of technical issue by my side . So how about i commit by making all the changes you suggest and then commit them.

@garg3133
Copy link
Owner

Okay @nitinjaiswal7537, you can do that. But how would you be able to test those changes? Maybe try hard-reloading your page (Ctrl + F5).

Also, don't forget about other changes I suggested.

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.

2 participants