Add navbar #5
Add navbar #5
Conversation
RobertoValle31
requested review from
lardissone,
mattvv,
shc023,
Kiri23,
jansen0825 and
dervin10
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/opticpower/tracker/5d2tua6m5 |
|
I think this is a great time for us to align on which |
|
https://styled-components.com/ might be a good option |
components/Nav.tsx
Outdated
| import { Row, Col, Spacer, Select } from '@geist-ui/react'; | ||
| import { Sun, Moon } from '@geist-ui/react-icons'; | ||
|
|
||
| const opticLogo = '/images/OpticPower.png'; |
There was a problem hiding this comment.
You may want to conditionally include a white logo depending on the useLight variable
There was a problem hiding this comment.
That's what I wanted initially, but I dint found the white logo. So I added a white shadow. It will be great if you could pass me the white version of the logo.
There was a problem hiding this comment.
@RobertoValle31 did you check the File Cabinet?
There was a problem hiding this comment.
All the folders have the logos but the Optic Power Logos folder is empty. Maybe I can use the codepwr ? Let me know what you think.
There was a problem hiding this comment.
@RobertoValle31 thanks for bringing this up, there are some sharing issues there, i'll try and resolve but for now you should have access
| { | ||
| "presets": ["next/babel"], | ||
| "plugins": [["styled-components", { "ssr": true }]] | ||
| } |
There was a problem hiding this comment.
I was having problems with the styled-components because every time I refreshed the page the component was losing all its CSS styles and I was having a warning on the browser console regarding the use of className although I was not using it. After research and testing different solutions this was the only one that worked vercel/next.js#4068 (comment) I had to create this file with this config.
| </Col> | ||
| <SelectContainer> | ||
| <Select placeholder={useLight ? 'Light Mode' : 'Dark Mode'}> | ||
| <Select.Option value="Dark Mode" onClick={(): void => setUseLight(false)}> |
There was a problem hiding this comment.
nit: why onClick on each option instead of onChange on the Select?
I had used something like:
<Select placeholder={useLight ? 'Light Mode' : 'Dark Mode'} onChange={(ev) => setLight(ev.target.value)}>
<Select.Option value="false">
<Moon size={16} /> <Spacer inline x={0.35} />
Dark Mode
</Select.Option>
{/* ... */}
</Select>Just curious.
There was a problem hiding this comment.
Hey, @lardissone I was trying your suggestion but I believe that you do that on MaterialUI, and works perfectly. If that is the case the prop value on MUI it can be any type. But on react-geist, the type of value can only be a string. The only way I was able to make this work was by doing the following:
<Select placeholder={useLight ? 'Light Mode' : 'Dark Mode'} onChange={value => setUseLight(value === 'true')}>
<Select.Option value="false">
<Moon size={16} /> <Spacer inline x={0.35} />
Dark Mode
</Select.Option>
<Select.Option value="true">
<Sun size={16} /> <Spacer inline x={0.35} />
Light Mode
</Select.Option>
</Select>Let me know what you think.
Description
This PR will update the navbar design. I just gave some simple changes like adding the OpticPower logo, bottom border, and changed the theme selection to a
Select.NOTE: This is my first time using TypeScript, please correct me if I'm doing something wrong on the syntax. Please let me know what you think, I'm open to suggestions. Thanks in advance.
Demo