-
Notifications
You must be signed in to change notification settings - Fork 73
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 twitter share logo and simple css proerty change #49
base: main
Are you sure you want to change the base?
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/anup-a/svgwave/H2ovYRfAf8ouzUBpJR4vBJKFWkM5 [Deployment for 4d04c45 failed] |
@anup-a Please check out this pull request |
@Nikhil27b Hey thanks for the PR. I'm sorry, busy atm. I'll review it by tomorrow. |
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.
I reviewed the code. Thanks for the PR. Would you please take a look at commets
@@ -20,9 +20,11 @@ | |||
"author": "Anup Aglawe", | |||
"license": "ISC", | |||
"dependencies": { | |||
"postcss": "^8.3.8", |
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.
Why did you add postcss in deps?
|
||
export default CustomBar | ||
|
||
import { h } from 'preact' |
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.
something looks messed up here?
@@ -17,6 +21,21 @@ function Navbar({ isDark, toggleDarkMode, color }) { | |||
</div> | |||
</div> | |||
<div className="flex items-center nav-item "> | |||
<div style={{marginRight: '0.5rem', marginTop: '0.5rem' }}> |
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.
- No responsive/mobile friendly styles.
- And logo doesn't go with the theme (a black outline twitter logo would look much better).
please review if you want to add more UX changes then comment I am good at frontend UX and UI.
I added a twitter share logo for the website and some CSS property changes