Merge pull request #319 from alexeykostevich/refactoring/es-header

Simplify `EsHeaderComponent` implementation.

Author: mansona

addon/components/es-header.js

@@ -1,43 +1,23 @@
-import { set, action } from '@ember/object';
 import Component from '@glimmer/component';
+import { action, set } from '@ember/object';
 
 import defaultLinks from '../constants/links';
 
+const defautHomePage = 'https://www.emberjs.com';
+
 export default class EsHeaderComponent extends Component {
   expanded = false;
 
   get navHome() {
-    if (this.args.home) {
-      return this.args.home;
-    }
-
-    return 'https://www.emberjs.com';
+    return this.args.home ?? defautHomePage;
   }
 
   get navLinks() {
-    if (this.args.links) {
-      return this.args.links;
-    }
-
-    return defaultLinks;
+    return this.args.links ?? defaultLinks;
   }
 
   @action
-  toggleMenu() {
+  onTogglerClick() {
     set(this, 'expanded', !this.expanded);
   }
-
-  trackExpanded() {
-    // Ensure menu is marked as expanded if there is enough screen estate
-    // TODO: Dynamically calculate necessary horizontal space and collapse based on that
-    if (typeof FastBoot === 'undefined') {
-      const mq = matchMedia('(min-width: 992px)');
-
-      mq.addListener(event => {
-        if (event.matches) {
-          set(this, 'expanded', false);
-        }
-      });
-    }
-  }
 }

addon/styles/components/es-header.css

@@ -59,11 +59,6 @@
   color: var(--color-white);
 }
 
-[aria-expanded="true"] ~ .navbar-list,
-[aria-expanded="true"] ~ .navbar-end {
-  display: block;
-}
-
 .navbar-toggler {
   display: block;
   width: 3rem;
@@ -79,6 +74,11 @@
   border-radius: var(--radius-lg);
 }
 
+.navbar-expanded .navbar-list,
+.navbar-expanded .navbar-end {
+  display: block;
+}
+
 .navbar-list-item {
   position: relative;
 }
@@ -186,7 +186,8 @@
     margin-right: var(--spacing-4);
   }
 
-  .navbar-list {
+  .navbar-list,
+  .navbar-expanded .navbar-list {
     display: flex;
     margin-top: 0;
   }
@@ -232,9 +233,6 @@
     z-index: 100;
   }
 
-  .navbar-dropdown-list-item-link {
-  }
-
   .navbar-end {
     display: block;
     align-self: center;

addon/templates/components/es-header.hbs

@@ -1,5 +1,9 @@
-<header class="es-header" role="banner" ...attributes>
-  <nav class="es-navbar">
+<header
+  class="es-header"
+  role="banner"
+  ...attributes
+>
+  <nav class="es-navbar {{if this.expanded "navbar-expanded"}}">
     <a class="navbar-brand-wrapper" href={{this.navHome}}>
       <img
         class="navbar-brand"
@@ -11,13 +15,12 @@
     </a>
 
     <button
+      class="navbar-toggler"
       type="button"
-      class="navbar-toggler ember-view"
       aria-expanded={{if this.expanded "true" "false"}}
-      {{did-insert this.trackExpanded}}
-      {{on "click" this.toggleMenu}}
+      {{on "click" this.onTogglerClick}}
     >
-      Show Site Navigation
+      {{if this.expanded "Hide" "Show"}} Site Navigation
     </button>
 
     <ul class="navbar-list">

tests/integration/components/es-header-test.js

@@ -1,43 +1,69 @@
-import { render } from '@ember/test-helpers';
+import { click, render } from '@ember/test-helpers';
+import { setProperties } from '@ember/object';
 import { module, test } from 'qunit';
 import { setupRenderingTest } from 'ember-qunit';
 import hbs from 'htmlbars-inline-precompile';
 
+const customHomeUrl = 'https://github.com/emberjs';
+
 module('Integration | Component | es header', function(hooks) {
   setupRenderingTest(hooks);
 
-  test('it renders', async function(assert) {
-    // Set any properties with this.set('myProperty', 'value');
-    // Handle any actions with this.on('myAction', function(val) { ... });
+  test('it uses the header HTML element with correct attributes', async function(assert) {
+    await render(hbs`<EsHeader/>`);
+
+    assert.dom('header').exists({ count: 1 });
+    assert.dom('header').hasClass('es-header');
+    assert.dom('header').hasAttribute('role', 'banner');
+  });
+
+  test('it renders the logo', async function(assert) {
+    await render(hbs`<EsHeader/>`);
 
-    await render(hbs`{{es-header}}`);
+    assert.dom('.navbar-brand').hasAttribute('src', '/images/ember-logo.svg');
+  });
+
+  test('it renders the link to the Ember homepage by default', async function(assert) {
+    await render(hbs`<EsHeader/>`);
+
+    assert.dom('.navbar-brand-wrapper').hasAttribute('href', 'https://www.emberjs.com');
+  });
 
-    assert.dom(this.element).containsText('Show Site Navigation');
+  test('it renders a link to a custom homepage', async function(assert) {
+    setProperties(this, { customHomeUrl });
+    await render(hbs`<EsHeader @home={{customHomeUrl}} />`);
 
-    // Template block usage:
+    assert.dom('.navbar-brand-wrapper').hasAttribute('href', customHomeUrl);
+  });
+
+  test('it renders custom content at the end', async function(assert) {
     await render(hbs`
-      {{#es-header}}
-        template block text
-      {{/es-header}}
+      <EsHeader>
+        Custom content
+      </EsHeader>
     `);
 
-    assert.dom(this.element).containsText('template block text');
+    assert.dom('.navbar-end').containsText('Custom content');
   });
 
-  test('it uses the header html element', async function(assert) {
-    await render(hbs`{{es-header}}`);
-    assert.dom('header').exists({ count: 1 }, 'the header uses the header html element!');
-  });
+  test('it renders the navbar collapsed by default', async function(assert) {
+    await render(hbs`<EsHeader/>`);
 
-  test('it has the role of banner', async function(assert) {
-    await render(hbs`{{es-header}}`);
-    assert.dom('header').hasAttribute('role', 'banner', 'header has the role of banner');
+    assert.dom('.es-navbar').doesNotHaveClass('navbar-expanded');
+    assert.dom('.navbar-toggler').containsText('Show Site Navigation');
   });
 
-  //the class matches the component name
-  //but how do I make it so it just checks for the one of them?
-  test('it has the className es-header', async function(assert) {
-    await render(hbs`{{es-header}}`);
-    assert.dom('header').hasClass('es-header');
+  test('it toggles the navbar when click the toggler', async function(assert) {
+    await render(hbs`<EsHeader/>`);
+
+    await click('.navbar-toggler');
+
+    assert.dom('.es-navbar').hasClass('navbar-expanded');
+    assert.dom('.navbar-toggler').containsText('Hide Site Navigation');
+
+    await click('.navbar-toggler');
+
+    assert.dom('.es-navbar').doesNotHaveClass('navbar-expanded');
+    assert.dom('.navbar-toggler').containsText('Show Site Navigation');
   });
 });