fix(cxl-ui): fix menu item padding, clickable area, nav rendering bug

conversionxl · Oct 26, 2023 · 9e836c9 · 9e836c9
1 parent 787818a
commit 9e836c9
Show file tree

Hide file tree

Showing 8 changed files with 100 additions and 68 deletions.
diff --git a/packages/cxl-lumo-styles/scss/themes/vaadin-context-menu-item.scss b/packages/cxl-lumo-styles/scss/themes/vaadin-context-menu-item.scss
@@ -7,6 +7,10 @@
   }
 }
 
+:host([theme~="cxl-marketing-nav"].vaadin-menu-item.has-link) {
+  cursor: pointer !important;
+}
+
 :host([theme~="cxl-marketing-nav"].section-header) {
   font-weight: 600;
   color: var(--lumo-body-text-color);

diff --git a/packages/cxl-lumo-styles/scss/themes/vaadin-context-menu-list-box.scss b/packages/cxl-lumo-styles/scss/themes/vaadin-context-menu-list-box.scss
@@ -14,20 +14,15 @@
 }
 
 :host([theme~="cxl-marketing-nav"].vaadin-menu-list-box) {
-  padding: var(--lumo-space-m);
+  padding: var(--lumo-space-m) 0;
 
   @media (min-width: 568px) {
     padding: 0;
-
-    [part="items"] ::slotted(.vaadin-menu-item) {
-      padding-right: var(--lumo-space-s);
-      padding-left: var(--lumo-space-s);
-    }
   }
 
   [part="items"] ::slotted(.vaadin-menu-item) {
-    padding-right: var(--lumo-space-s);
-    padding-left: var(--lumo-space-s);
+    padding-right: var(--lumo-space-m);
+    padding-left: var(--lumo-space-m);
   }
 
   [part="items"]

diff --git a/packages/cxl-lumo-styles/scss/themes/vaadin-menu-bar.scss b/packages/cxl-lumo-styles/scss/themes/vaadin-menu-bar.scss
@@ -10,7 +10,7 @@
     [part="overflow-button"] {
 
       vaadin-icon {
-        &.close-icon {
+        &.vaadin-menu-bar-button--icon {
           display: none;
         }
       }
@@ -24,6 +24,14 @@
 
         vaadin-icon {
           height: var(--lumo-icon-size-s);
+
+          &.vaadin-menu-bar-button--icon {
+            display: block;
+
+            &.close-icon {
+              display: none;
+            } 
+          }
 
           svg {
             fill: var(--lumo-shade);

diff --git a/packages/cxl-lumo-styles/scss/themes/vaadin-overlay.scss b/packages/cxl-lumo-styles/scss/themes/vaadin-overlay.scss
@@ -54,7 +54,7 @@
   }
 
   &::part(content) {
-    padding: var(--lumo-space-s) var(--lumo-space-m);
+    padding: var(--lumo-space-s);
   }
 
   @media (max-width: 568px), (max-height: 568px) {

diff --git a/packages/cxl-ui/scss/cxl-marketing-nav.scss b/packages/cxl-ui/scss/cxl-marketing-nav.scss
@@ -23,7 +23,7 @@
     max-width: 100vw;
     padding: 0 var(--lumo-space-m);
 
-    &[minimal] {
+    &:not([wide]) {
       vaadin-menu-bar {
         max-width: 64px;
       }
@@ -54,6 +54,18 @@
       width: 100%;
     }
 
+    &[minimal] {
+
+      vaadin-menu-bar {
+
+        &::part(overflow-button) {
+          visibility: hidden;
+          pointer-events: none;
+        }
+      }
+    }
+
+
     ::part(menu-bar-button),
     ::slotted(.menu-item),
     .menu-item {

diff --git a/packages/cxl-ui/src/components/cxl-marketing-nav.js b/packages/cxl-ui/src/components/cxl-marketing-nav.js
@@ -1,3 +1,4 @@
+/* eslint-disable yoda */
 /* eslint-disable import/no-extraneous-dependencies */
 import { LitElement, html } from 'lit';
 import { customElement, property, state, query, queryAll } from 'lit/decorators.js';
@@ -9,6 +10,8 @@ import cxlMarketingNavStyles from '../styles/cxl-marketing-nav-css.js';
 import cxlMarketingNavGlobalStyles from '../styles/global/cxl-marketing-nav-css.js';
 import '@vaadin/icon';
 import '@vaadin/menu-bar';
+import '@vaadin/context-menu';
+import '@vaadin/button';
 import '@vaadin/dialog';
 
 @customElement('cxl-marketing-nav')
@@ -56,17 +59,25 @@ export class CXLMarketingNavElement extends LitElement {
         newItem.children = [{ component: 'back' }, ...item.children].map(parseItem);
       };
 
+      if ('primary' === group) {
+        newItem.separator = true;
+      }
+
       return newItem;
     };
 
     const groups = [];
 
     Object.keys(data).forEach((group) => {
       const items = data[group];
-      groups.push({
+      const newGroup = {
         name: group,
         items: [...items?.map((item) => parseItem(item, group))],
-      });
+      };
+      if ('primary' === group) {
+        newGroup.items.push({ component: this._createSearchButton(), isSearch: true });
+      }
+      groups.push(newGroup);
     });
 
     this.groups = groups;
@@ -81,14 +92,15 @@ export class CXLMarketingNavElement extends LitElement {
   @property({ type: Array })
   get mobileGroups() {
     // eslint-disable-next-line no-nested-ternary
-    const sorter = (a, b) => (a.name === 'global' ? 1 : b.name === 'global' ? -1 : 0);
+    const sorter = (a, b) => ('global' === a.name ? 1 : 'global' === b.name ? -1 : 0);
+    const groups = [...this.groups];
 
     return [
       {
         name: 'primary',
-        items: this.groups
+        items: groups
           .sort(sorter)
-          .map((group) => group.items)
+          .map((group) => group.items.filter((item) => !item.isSearch))
           .flat(1),
       },
     ];
@@ -135,32 +147,31 @@ export class CXLMarketingNavElement extends LitElement {
   render() {
     // collapse all navs into one if in mobile layout
     const groups = this.wide ? this.groups : this.mobileGroups;
-
+    
     return html`
       ${groups.map((group) => {
         const { name, items } = group;
 
         return html`
-          <nav id="menu-${name}-items" ?minimal=${!this.wide}>
+          <nav id="menu-${name}-items" ?minimal=${this.minimal} ?wide=${this.wide}>
             <div class="container">
-              ${name === 'global' || !this.wide
+              ${'global' === name || !this.wide
                 ? html`
-                    <vaadin-context-menu-item class="cxl-logo" theme="tertiary cxl-marketing-nav">
+                    <vaadin-menu-bar-button class="cxl-logo" theme="tertiary cxl-marketing-nav">
                       <a href=${this.homeUrl || 'https://cxl.com'}>
                         <vaadin-icon
                           icon="cxl:logo"
                           style="width: var(--lumo-icon-size-xl, 48px);"
                         ></vaadin-icon>
                       </a>
-                    </vaadin-context-menu-item>
+                    </vaadin-menu-bar-button>
                   `
                 : ''}
                 
               ${!this.wide ? this._renderSearch(group) : ''}
-              <vaadin-menu-bar theme="tertiary cxl-marketing-nav" .items=${items}></vaadin-menu-bar>
+              <vaadin-menu-bar id="${name}-menu-bar" theme="tertiary cxl-marketing-nav" .items=${items}></vaadin-menu-bar>
               <slot name="${name}-after"></slot>
               <slot name="${name}-end"></slot>
-              ${this.wide ? this._renderSearch(group) : ''}
             </div>
           </nav>
         `;
@@ -200,23 +211,30 @@ export class CXLMarketingNavElement extends LitElement {
 
   // eslint-disable-next-line class-methods-use-this
   _renderSearch(group) {
-    if (this.minimal || group.name !== 'primary') return '';
+    if (this.minimal || 'primary' !== group.name) return '';
 
     return html`
       <vaadin-menu-bar-button
         theme="tertiary cxl-marketing-nav"
         class="search-button"
-        @click=${this.toogleSearchDialog}
+        @click=${this.toggleSearchDialog}
       >
-        <a>
+        <span>
           <vaadin-icon icon="lumo:search"></vaadin-icon> Search
           <vaadin-icon icon="lumo:dropdown"></vaadin-icon>
-        </a>
+        </span>
       </vaadin-menu-bar-button>
     `;
   }
 
-  toogleSearchDialog() {
+  _createSearchButton() {
+    const searchMenuItemElement = this.createItem({ text: 'Search', icon: 'search' }, 'primary');
+    searchMenuItemElement.addEventListener('click', this.toggleSearchDialog.bind(this));
+
+    return searchMenuItemElement;
+  }
+
+  toggleSearchDialog() {
     this.searchDialogOpen = !this.searchDialogOpen;
   }
 
@@ -231,20 +249,26 @@ export class CXLMarketingNavElement extends LitElement {
     [...document.body.querySelectorAll('vaadin-context-menu-overlay')].at(-1).close();
   }
 
+  // eslint-disable-next-line class-methods-use-this
+  _redirectMenuItemClick(e)  {
+    const { currentTarget } = e;
+    const anchorElement = currentTarget.querySelector('a');
+    anchorElement.click();
+  }
+
   // Creates and returns a custom menu item component for use in vaadin-menu-bar
   createItem(
     { text, description, sectionheader, component, icon, href, children, depth, classes },
     group
   ) {
     // If divider, return an <hr> element, nothing else.
-    if (component === 'hr') {
+    if ('hr' === component) {
       return document.createElement('hr');
     }
-
     // Create menu item.
     const menuItemElement = document.createElement('vaadin-context-menu-item');
 
-    // Add relevant classes conditionally.
+    // Add relevant classes and attributes conditionally.
     if (group) {
       menuItemElement.classList.add(`${group}-menu-item`);
     }
@@ -264,13 +288,23 @@ export class CXLMarketingNavElement extends LitElement {
       prefixIconElement.classList.add('vaadin-context-menu-item--icon');
       menuItemElement.appendChild(prefixIconElement);
     }
-
-    // Add regular link or text label.
+    
+    // Add regular link, text label or CTA.
     if (href && !children && !sectionheader) {
       const link = document.createElement('a');
+      const isCTA = classes?.includes('menu-item-cta')
       link.href = href;
-      link.innerText = text;
+      if (isCTA) {
+        const ctaElement = document.createElement('vaadin-button');
+        ctaElement.setAttribute('theme', 'primary');
+        ctaElement.innerText = text;
+        link.appendChild(ctaElement);
+      } else {
+        link.innerText = text;
+      }
       menuItemElement.appendChild(link);
+      menuItemElement.addEventListener('click', this._redirectMenuItemClick);
+      menuItemElement.classList.add('has-link');
     } else if (text) {
       const labelElement = sectionheader
         ? document.createElement('h6')
@@ -289,15 +323,15 @@ export class CXLMarketingNavElement extends LitElement {
     }
 
     // Add suffix dropdown icon, for a top level item with children.
-    if (children?.length && depth === 0) {
+    if (children?.length && 0 === depth) {
       const suffixIconElement = document.createElement('vaadin-icon');
       suffixIconElement.setAttribute('icon', 'lumo:dropdown');
       suffixIconElement.classList.add('vaadin-context-menu-item--dropdown-icon');
       menuItemElement.appendChild(suffixIconElement);
     }
 
-    // /Add back button. Used in mobile layout only.
-    if (component === 'back') {
+    // Add back button. Used in mobile layout only.
+    if ('back' === component) {
       const backButtonElement = document.createElement('vaadin-button');
 
       backButtonElement.classList.add('context-menu-item-back-button');
@@ -335,7 +369,6 @@ export class CXLMarketingNavElement extends LitElement {
       overflowMenuButton.toggleAttribute('hidden', false);
 
       if (overflowMenuButton && !overflowMenuButton.iconFixed) {
-
         const menuIcon = document.createElement('vaadin-icon');
         menuIcon.setAttribute('icon', 'lumo:menu');
         menuIcon.classList.add('vaadin-menu-bar-button--icon');

diff --git a/packages/storybook/cxl-ui/cxl-marketing-nav.data.json b/packages/storybook/cxl-ui/cxl-marketing-nav.data.json
@@ -49,6 +49,14 @@
       "href": "https://cxl.com/live",
       "id": 96,
       "parent": 0
+    },
+    {
+      "depth": 0,
+      "id": 51595,
+      "parent": 0,
+      "href": "https://cxl.com/institute/pricing/",
+      "classes": ["menu-item-cta", "menu-item-split-nav", "menu-item-start-free-trial"],
+      "text": "Sign up now"
     }
   ],
   "primary": [

diff --git a/packages/storybook/cxl-ui/cxl-marketing-nav.stories.js b/packages/storybook/cxl-ui/cxl-marketing-nav.stories.js
@@ -10,38 +10,15 @@ export default {
 
 window.Headroom = Headroom;
 
-export const CXLMarketingNav = (args) => {
+export const CXLMarketingNav = () => {
   useEffect(() => {
     // Populate `<cxl-marketing-nav>` context menus.
     const cxlMarketingNavElement = document.querySelector('cxl-marketing-nav');
 
     cxlMarketingNavElement.contextMenuItems = contextMenuItems;
-
-    const htmlEl = document.querySelector('html');
-    if (args?.loggedIn) {
-      htmlEl.classList.add('loggedIn');
-    } else {
-      htmlEl.classList.remove('loggedIn');
-    }
-  }, [args?.loggedIn]);
+  }, []);
 
   return html`
-    <style>
-      html.loggedIn {
-        margin-top: 32px;
-      }
-
-      #wp-admin-bar {
-        position: fixed;
-        width: 100%;
-        height: 32px;
-        background-color: gray;
-        top: 0;
-        color: white;
-        padding: 0 1em;
-      }
-    </style>
-
     <cxl-marketing-nav class="menu" role="navigation" slot="header">
       <div id="search-form-container">
         <form id="search-form" role="search" method="get" class="search-form" action="https://cxl.com/institute/?s=">
@@ -61,10 +38,5 @@ export const CXLMarketingNav = (args) => {
         </form>
       </div>
     </cxl-marketing-nav>
-    <div id="wp-admin-bar" ?hidden=${!args?.loggedIn}>wp-admin bar: test nav menu placement</div>
   `;
 };
-
-CXLMarketingNav.args = {
-  loggedIn: true,
-};