DEV: Import nav component #59
Conversation
| $screen-lg-min: 62em; // 992px | ||
|
|
||
| // Large tablets and desktops | ||
| $screen-xl-min: $nav-minimum-display-width; // 1440px |
There was a problem hiding this comment.
This doesn't appear to be defined anywhere
| <a | ||
| href="#" | ||
| class="navigation-list__link navigation-list__link--has-child" | ||
| > | ||
| About | ||
| {{dIcon "chevron-down" class="navigation-list__chevron"}} | ||
| </a> | ||
| <ul | ||
| class="navigation-list__sub-item-list js-dropdown-list | ||
| {{if (eq this.currentSubmenuIndex 0) 'is-active'}}" | ||
| aria-hidden="true" | ||
| aria-label="Resources submenu" | ||
| > | ||
| <li class="navigation-list__sub-item"> | ||
| <a href="https://discourse.org/about"> | ||
| What is Discourse? | ||
| </a> | ||
| </li> | ||
| <li class="navigation-list__sub-item"> | ||
| <a href="https://discourse.org/team"> | ||
| Who we are | ||
| </a> | ||
| </li> | ||
| <li class="navigation-list__sub-item"> | ||
| <a href="https://discourse.org/customers"> | ||
| Customers | ||
| </a> | ||
| </li> | ||
| <li class="navigation-list__sub-item"> | ||
| <a href="https://discourse.org/wall-of-love"> | ||
| Wall of love | ||
| </a> | ||
| </li> | ||
| <li class="navigation-list__sub-item"> | ||
| <a href="https://discourse.org/partners"> | ||
| Partners | ||
| </a> | ||
| </li> | ||
| <li class="navigation-list__sub-item"> | ||
| <a href="https://discourse.org/jobs"> | ||
| Careers | ||
| </a> | ||
| </li> | ||
| </ul> | ||
| </li> | ||
| <li class="navigation-list__item"> | ||
| <a | ||
| href="https://discourse.org/features" | ||
| class="navigation-list__link" | ||
| > | ||
| Features | ||
| </a> | ||
| </li> | ||
| <li class="navigation-list__item"> | ||
| <a | ||
| href="https://discover.discourse.com" | ||
| class="navigation-list__link is-active" | ||
| > | ||
| Discover | ||
| </a> | ||
| </li> | ||
| <li class="navigation-list__item"> | ||
| <a | ||
| href="https://discourse.org/enterprise" | ||
| class="navigation-list__link" | ||
| > | ||
| Enterprise | ||
| </a> | ||
| </li> | ||
| <li class="navigation-list__item"> | ||
| <a | ||
| class="navigation-list__link" | ||
| href="https://discourse.org/pricing" | ||
| ga-on="click" | ||
| ga-event-category="main-nav" | ||
| ga-event-label="pricing" | ||
| ga-event-action="click" | ||
| > | ||
| Pricing | ||
| </a> | ||
| </li> | ||
| <li class="navigation-list__item"> | ||
| <a href="#" class="navigation-list__link"> | ||
| Resources | ||
| </a> | ||
| </li> | ||
| </ul> |
There was a problem hiding this comment.
Should we make this configurable in the UI settings for easy updating in the future if links need to be added or changed?
| <ul | ||
| class="navigation-list__sub-item-list js-dropdown-list | ||
| {{if (eq this.currentSubmenuIndex 0) 'is-active'}}" | ||
| aria-hidden="true" |
There was a problem hiding this comment.
I believe this should be dynamic based on whether the submenu is visible or not
| @action | ||
| submenuMouseEnter() { | ||
| this.isSubmenuHovered = true; | ||
| if (this.hideSubmenuTimer) { | ||
| cancel(this.hideSubmenuTimer); | ||
| } | ||
| } | ||
|
|
||
| @action | ||
| submenuMouseLeave() { | ||
| this.isSubmenuHovered = false; | ||
| if (!this.isTopLevelMenuItemHovered) { | ||
| this.scheduleSubmenuHide(); | ||
| } | ||
| } |
| @action | ||
| hideSubmenu() { | ||
| this.scheduleSubmenuHide(); | ||
| } |
There was a problem hiding this comment.
This doesn't appear to be used anywhere
|
|
||
| export default class HeaderNav extends Component { | ||
| @tracked currentSubmenuIndex = null; | ||
| @tracked hideSubmenuTimer = null; |
There was a problem hiding this comment.
We should add a willDestroy hook and return this back to null to ensure the timer is destroyed on component destroy
pmusaraj
left a comment
pmusaraj
left a comment
There was a problem hiding this comment.
Given the current nav is in a non-version-controlled component, I'm OK with making this change now.
Can you post mobile screenshots as well in the PR description @derekrushforth?
Thanks!
|
@pmusaraj I've posted some screenshots of desktop and mobile in the description. It looks like we currently hide the nav on mobile. I plan on doing a follow up PR that should fix this as well as apply new styles for the upcoming design changes. @keegangeorge Thanks a bunch for your help. I'll go through and clean out the unused code before we release this. |
* Remove unused methods * Reset timer when component is destroyed * Set aria on submenu dynamically
|
@keegangeorge Let me know if these changes look good to you: |
This PR imports the nav component into the theme. This is currently a static theme component managed on the instance. Keeping it in the theme should make it easier to maintain.
This also converts the nav into a Glimmer component and syncs the nav items so it matches the marketing site.
Desktop
Mobile
Hiding the nav on mobile is how it's currently being done in production.