markbello / gatsbyjs-nyc

Official Website for the Gatsby NYC Meetup
MIT License
10 stars 7 forks source link

feat(NAVBAR): update navbar #10

Closed eddgr closed 5 years ago

eddgr commented 5 years ago
olegchursin commented 5 years ago

Changed the PR title to somewhat follow Conventional Commits: https://www.conventionalcommits.org/en/v1.0.0-beta.2/

Marked used CONTRIBUTING to denote addition of a feature to CONTRIBUTE specs. Since you are working on navbar, hence the change.

olegchursin commented 5 years ago

Love the use of styled components!

markbello commented 5 years ago

good work! thanks @eddgr

im curious about having separate components for regular vs mobile nav, since the code looks pretty much the same between both components. would we be able to consolidate these into one component by using media queries inside the styled components? (https://jsramblings.com/2018/02/04/styled-components-media-queries.html)

also, i'm wondering if we should pull the styled components into a single styled.js file per https://stackoverflow.com/questions/42987939/styled-components-organization in order to cut down on some of the noise. i'd wager this could expose some duplicate code to dry up