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

[Review Needed] Fixing of CSS and JS and adding internal links #641

Closed
wants to merge 1 commit into from
Closed

Conversation

roninx991
Copy link
Contributor

@roninx991 roninx991 commented Dec 25, 2018

  • Included a Preview link and screenshot showing after and before the changes.
  • Included a description of the change below.
  • Squashed the commits.

Changes done in this Pull Request

Description / Changes

1. Addition of jquery-

Internal jquery has been added as many libraries depended on it. However addition of internal jquery induced some new issues such as broken slider and navbar issues.

2. Navbar Fix-

Fixed navbar according to theme.

3. CSS Fix-

Fixed broken css and fontawesome

4. Image Slider Fix-

Fixed broken image slider using bxslider library instead of flexslider which confilcted with code.

Screenshots if any:

Before:

screenshot from 2018-12-23 16-12-22

screenshot from 2018-12-23 17-01-42

screenshot from 2018-12-23 16-12-38

screenshot from 2018-12-23 16-12-44

screenshot from 2018-12-23 16-12-47

screenshot from 2018-12-23 16-12-51

After:

screenshot from 2018-12-25 18-39-06

screenshot from 2018-12-23 16-13-33

screenshot from 2018-12-23 16-13-50

screenshot from 2018-12-23 16-13-55

screenshot from 2018-12-23 16-13-58

screenshot from 2018-12-23 16-14-03


Copy link
Member

@NishithP2004 NishithP2004 left a comment

Choose a reason for hiding this comment

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

c301f684-02b9-408e-a767-3ffb5f2f4474
59be8d3a-7baf-4950-8aba-a1c0253c070a
Uploading 3DDAF152-CB39-4D86-B37F-17D2EFF966CD.png…
Uploading AE8E4853-61B6-42F8-BB23-EAB8E78D8EB9.png…

@@ -97,17 +97,17 @@ <h6 class="title">Latest Updates</h6>
</li>
<li>
<a href="https://gitter.im/fossasia/fossasia" target="_blank" rel="noopener noreferrer">
<i class="fab fa-gitter"></i>
<i class="fa fa-comment"></i>
Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove this ?

Copy link
Member

Choose a reason for hiding this comment

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

Why has Gitter been removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have used fontawesome4.7 and it didn't have gitter icon. The link is working only the icon has been changed. I did this because fontawesome5 was giving garbage icons when used internally.

Copy link
Member

Choose a reason for hiding this comment

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

But Gitter is required

Copy link
Member

Choose a reason for hiding this comment

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

Please use frontawesome 5 only.

Choose a reason for hiding this comment

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

The link to gitter should have the gitter icon

</a>
</li>
<li>
<a href="https://www.weibo.com/u/5784920339" target="_blank" rel="noopener noreferrer">
<i class="fab fa-weibo"></i>
<i class="fa fa-weibo"></i>
Copy link
Member

Choose a reason for hiding this comment

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

Why is this changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, I have used fontawesome 4.7 instead of fontawesome 5 since the later was not giving the expected icons

Copy link
Member

Choose a reason for hiding this comment

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

Please use frontawesome 5 only

Choose a reason for hiding this comment

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

The icon is correct and works for weibo, hence these changes are justified
Also font awesome 5 icon for weibo is not working

Copy link
Member

Choose a reason for hiding this comment

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

Please use font awesome 5 as Instagram icon is not working and Gitter should have Gitter icon.

</a>
</li>
<li>
<a href="https://vk.com/fossasia" target="_blank" rel="noopener noreferrer">
<i class="fab fa-vk"></i>
<i class="fa fa-vk"></i>
Copy link
Member

Choose a reason for hiding this comment

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

Why is this altered?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fontawesome 4.7 uses fa instead of fab

Copy link
Member

Choose a reason for hiding this comment

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

Please use frontawesome 5 only.

@NishithP2004
Copy link
Member

Why has the js effect for the **Thank You For Contributing ** section disappeared.Please squash your commits.Why does the GitHub links of the students and mentors appear below the image?

@NishithP2004
Copy link
Member

Requesting a review @mariobehling @jogendra @nikit19

@roninx991
Copy link
Contributor Author

screenshot from 2018-12-28 01-30-12
screenshot from 2018-12-28 01-30-23

@NishithP2004 @pranav1698 Have added fontawesome 5 and removed fontawesome 4.7. Refer screenshots.

@roninx991 roninx991 changed the title Fixing of CSS and JS and adding internal links [Review Needed] Fixing of CSS and JS and adding internal links Dec 27, 2018
@NishithP2004
Copy link
Member

What has happened to the Thank you for contributing which should have a nice js effect.

@NishithP2004
Copy link
Member

Just a suggestion, is there any way you can make the icons etc red? @Sangatdas

@NishithP2004
Copy link
Member

Probably by changing the colour gradient.

@roninx991
Copy link
Contributor Author

What has happened to the Thank you for contributing which should have a nice js effect.

Can you provide me with some video or something? I hadn't seen the site before this bug was reported.

@pranav1698
Copy link

pranav1698 commented Dec 28, 2018 via email

@kksinghal
Copy link
Contributor

kksinghal commented Dec 29, 2018

You have missed custom.css . It can be added in this PR . It will fix current issues too . You can get that file from previous commits as mentioned by Pranav .

Copy link
Member

@rpotter12 rpotter12 left a comment

Choose a reason for hiding this comment

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

@Sangatdas please check this PR again :)

Copy link
Member

@NishithP2004 NishithP2004 left a comment

Choose a reason for hiding this comment

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

Please fix the Map , Contributors Section , Thank You For Contributing in various languages with a js effect

@NishithP2004
Copy link
Member

@Sangatdas Please do as requested for the PR to be merged.

Copy link

@pranav1698 pranav1698 left a comment

Choose a reason for hiding this comment

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

@Sangatdas You have to recheck all the changes that you are trying to do

@roninx991
Copy link
Contributor Author

I hadn't seen the previous site earlier before making the PR. Now, that I've seen it, I'll be able to make the required changes

@roninx991
Copy link
Contributor Author

I'll be closing this PR and request a new PR since I have to do a lot of changes. Thanks for the reviews and guidance

@roninx991 roninx991 closed this Jan 5, 2019
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.

None yet

5 participants