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

changed bottom ui; Using fa-icons know #23

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

pythongiant
Copy link
Member

@pythongiant pythongiant commented Jun 27, 2018

Using fa-icons know in place of previous SVGs, added animations and changed color of the bottom nav, added rel="icon" to webpage
screenshot from 2018-06-27 23-42-36

@gabru-md
Copy link
Member

please post a simple screenshot.
Thank You

@pythongiant
Copy link
Member Author

Nothing to worry about in the same 2 files above. Just messed up my workflow a bit

Copy link
Member

@gabru-md gabru-md left a comment

Choose a reason for hiding this comment

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

lgtm

@xeon-zolt
Copy link
Member

@pythongiant good work but do you know the negative impact of your changes ? its a simple light webpage now the loading time will increase cause we are requesting another server for the fonts either we make them local in the repo ( then we need to do some benchmarking to check what takes more time SVG or these fonts )
what do you think ?

@pythongiant
Copy link
Member Author

sure @xeon-zolt I am okay with whatever the benchmarks show.

Copy link
Member

@ParthS007 ParthS007 left a comment

Choose a reason for hiding this comment

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

@pythongiant Please resolve conflicts 👍

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.

4 participants