-
Notifications
You must be signed in to change notification settings - Fork 273
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
feat: 791-header-shell-links-removing-left-nav #848
feat: 791-header-shell-links-removing-left-nav #848
Conversation
This reverts commit 8d797b2.
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/carbon-design-system/gatsby-theme-carbon/oxsvj1x38 |
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.
Looking awesome, a couple minor things. I'd like for the hasHeaderNavigation to be useHeaderNavigation
and have it be a theme option folks can pass into their config. Let me know if you wanna work together on it!
packages/gatsby-theme-carbon/src/components/HeaderNav/HeaderNav.js
Outdated
Show resolved
Hide resolved
packages/gatsby-theme-carbon/src/components/LeftNav/LeftNavWrapper.module.scss
Outdated
Show resolved
Hide resolved
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.
Looking great, I think we can remove the option from the example now so it's back to the default.
packages/gatsby-theme-carbon/src/components/LeftNav/LeftNavWrapper.module.scss
Outdated
Show resolved
Hide resolved
done! |
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.
remove prop drilling pls
Awesome work, you crushed it! |
Closes #791