-
Notifications
You must be signed in to change notification settings - Fork 188
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
Search bar added to Navbar instead of participants #302
Conversation
de9df0e
to
06185b7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HTML and CSS looks good to me. Good job @ODAVING
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look really good, well done @ODAVING! Small nit-picky thing just before we merge. If you think the Inconsolata font is absolutely necessary, let me know and we can still merge. I just want to make sure we need it before we commit forever lol. Other than that, looks good to me.
src/frontend/style/main.css
Outdated
@@ -1,3 +1,6 @@ | |||
/* Font Used for Search Icon */ | |||
@import url('https://fonts.googleapis.com/css?family=Inconsolata:700'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe keep font dependencies to a minimum? Not only for a more consistent design but also to avoid loading a bunch (although this one is only ~60KB, but it adds up).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed it to the main font we use in HTML.
eslint fix changes removed ids for another iteration removed font for consistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good!
fixes #242
Added the search button as well as its animation to HTML/CSS. The button will transition between two states, as pictured below
FROM:
TO: