Tests & Breadcrumb improvements (#466)

* Add bootstrap -> bootstrap-vue migration notes

* add bFormFieldset label text alignment options

* fixes per @pi0

* add focus method proxied to element ref

* Popover dom node leak fix

* badge: add refs and generate variants

* improving error messages when matcher fn called improperly

* badge test suite

* propagate click event and emit click with event obj

* fix for dynamically setting the active link

Before the link would always set the last item of the array as the active one. However, the active state was read from the user submitted items.active prop, rather than the normalized item.__active.

Some more semantic improvements to make the code clearer

* formatting

* use only one lookup of array length

* two components and items lists for tests

* breadcrumb testing

* stop scroll to top behavior

__tests__/components/badge.spec.js

@@ -1,6 +1,42 @@
-import {loadFixture, testVM} from '../helpers';
+import { loadFixture, testVM } from '../helpers'
+
+const variantList = [
+    'default',
+    'primary',
+    'success',
+    'info',
+    'warning',
+    'danger',
+].map(variant => {
+    return { ref: `badge_${variant}`, variant }
+})
 
 describe('badge', async() => {
-    beforeEach(loadFixture('badge'));
-    testVM();
-});
+    beforeEach(loadFixture('badge'))
+    testVM()
+
+    it('should apply variant classes', async() => {
+        const { app: { $refs, $el } } = window
+
+        expect($refs.badge_pill).toHaveAllClasses(['badge', 'badge-pill'])
+
+        variantList.forEach(({ ref, variant }) => {
+            const vm = $refs[ref][0]
+            expect(vm).toHaveAllClasses(['badge', `badge-${variant}`])
+        })
+    })
+
+    it('should apply default pill class when not passed variant', async() => {
+        const { app: { $refs, $el } } = window
+
+        const vm = $refs.no_props
+        expect(vm).toHaveClass('badge-default')
+    })
+
+    it('should not apply pill class when not passed pill boolean prop', async() => {
+        const { app: { $refs, $el } } = window
+
+        const vm = $refs.no_props
+        expect(vm).not.toHaveClass('badge-pill')
+    })
+});

__tests__/components/breadcrumb.spec.js

@@ -1,6 +1,77 @@
-import {loadFixture, testVM} from '../helpers';
+import { loadFixture, testVM } from '../helpers'
 
 describe('breadcrumb', async() => {
-    beforeEach(loadFixture('breadcrumb'));
-    testVM();
-});
+    beforeEach(loadFixture('breadcrumb'))
+    testVM()
+
+    it('should apply bootstrap breadcrumb classes', async() => {
+        const { app: { $refs, $el } } = window
+        const vm = $refs.breadcrumb1
+        const $ol = vm.$el
+
+        expect($ol.classList.contains('breadcrumb')).toBe(true)
+
+        Array.from($ol.children).forEach($li => {
+            if ($li.tagName === 'LI') {
+                expect($li.classList.contains('breadcrumb-item')).toBe(true)
+            }
+        })
+    })
+
+    it('should apply ARIA roles', async() => {
+        const { app: { $refs, $el } } = window
+        const vm = $refs.breadcrumb1
+        const $ol = vm.$el
+
+        expect($ol.getAttribute('role')).toBe('navigation')
+
+        Array.from($ol.children).forEach($li => {
+            if ($li.tagName === 'LI') {
+                expect($li.getAttribute('role')).toBe('presentation')
+            }
+        })
+    })
+
+    it('should apply active class', async() => {
+        const { app: { $refs, $el } } = window
+        const vm = $refs.breadcrumb2
+        const $listItems = Array.from(vm.$el.children)
+
+        app.items2.forEach((item, i) => {
+            if (item.active) {
+                expect($listItems[i].classList.contains('active')).toBe(true)
+            }
+        })
+    })
+
+    it('should default active class to last item only when no true active prop provided', async() => {
+        const { app: { $refs, $el } } = window
+        const vm = $refs.breadcrumb1
+        const $listItems = Array.from(vm.$el.children)
+        const itemsLength = app.items.length
+
+        app.items.forEach((item, i) => {
+            const isLast = i === itemsLength - 1
+
+            if (isLast) {
+                expect($listItems[i].classList.contains('active')).toBe(true)
+            } else {
+                expect($listItems[i].classList.contains('active')).toBe(false)
+            }
+        })
+    })
+
+    it('should emit a click event with the item when clicked', async() => {
+        const { app: { $refs, $el } } = window
+        const vm = $refs.breadcrumb2
+        const spy = jest.fn();
+
+        vm.$on('click', spy)
+        const $listItems = Array.from(vm.$el.children)
+
+        app.items2.forEach((item, index) => {
+            $listItems[index].click()
+            expect(spy).toHaveBeenCalledWith(item)
+        })
+    })
+});

__tests__/helpers.js

@@ -8,7 +8,12 @@ const throwIfNotVueInstance = vm => {
     if (!vm instanceof Vue) {
         // debugging breadcrumbs in case a non-Vue instance gets erroneously passed
         // makes the error easier to fix than example: "Cannot read _prevClass of undefined"
-        throw new TypeError('The matcher `vmToHaveClasses` expects Vue instance.')
+        throw new TypeError(`The matcher function expects Vue instance. Given ${typeof vm}`)
+    }
+}
+const throwIfNotArray = array => {
+    if (!Array.isArray(array)) {
+        throw new TypeError(`The matcher requires an array. Given ${typeof array}`)
     }
 }
 
@@ -59,6 +64,7 @@ expect.extend({
     },
     toHaveAllClasses(vm, classList) {
         throwIfNotVueInstance(vm)
+        throwIfNotArray(classList)
 
         let pass = true;
         let missingClassNames = []

examples/badge/demo.html

@@ -1,5 +1,17 @@
 <div id="app">
-    <h3>Example heading <b-badge>New</b-badge></h3>
-    <h4>Example heading <b-badge variant="primary">New</b-badge></h4>
-    <h5>Example heading <b-badge pill variant="success">New</b-badge></h5>
+    <h3>Example heading
+        <b-badge ref="no_props">New</b-badge>
+    </h3>
+    <h4>Example heading
+        <b-badge variant="primary">New</b-badge>
+    </h4>
+    <h5>Example heading
+        <b-badge pill
+                 ref="badge_pill"
+                 variant="success">New</b-badge>
+    </h5>
+    <p v-for="variant in variants">
+        <b-badge :variant="variant"
+                 :ref="`badge_${variant}`">{{ variant }}</b-badge>
+    </p>
 </div>

examples/badge/demo.js

@@ -1,3 +1,15 @@
 window.app = new Vue({
-    el: '#app'
+    el: '#app',
+    data() {
+        return {
+            variants: [
+                'default',
+                'primary',
+                'success',
+                'info',
+                'warning',
+                'danger',
+            ]
+        }
+    },
 });

examples/breadcrumb/demo.html

@@ -1,3 +1,4 @@
 <div id="app">
-    <b-breadcrumb :items="items" />
+    <b-breadcrumb ref="breadcrumb1" :items="items" />
+    <b-breadcrumb ref="breadcrumb2" :items="items2" />
 </div>

examples/breadcrumb/demo.js

@@ -2,14 +2,30 @@ window.app = new Vue({
     el: '#app',
     data: {
         items: [{
+            text: 'Home',
+            link: 'https://bootstrap-vue.github.io'
+        }, {
             text: 'Admin',
-            link: '#'
+            to: '#',
+            active: false
         }, {
             text: 'Manage',
             link: '#'
         }, {
             text: 'Library',
+        }],
+        items2: [{
+            text: 'Home',
+            link: 'https://bootstrap-vue.github.io'
+        }, {
+            text: 'Admin',
+            link: '#',
             active: true
+        }, {
+            text: 'Manage',
+            link: '#'
+        }, {
+            text: 'Library',
         }]
-    }
+    },
 });

lib/components/breadcrumb.vue

@@ -1,17 +1,16 @@
 <template>
-    <ol class="breadcrumb" role="navigation">
-        <li v-for="item in items2"
-            :class="['breadcrumb-item', item.__active ? 'active' : null]"
-            @click="onclick(item)"
-            role="presentation"
-        >
-            <span v-if="item.active" v-html="item.text"></span>
+    <ol class="breadcrumb"
+        role="navigation">
+        <li v-for="item in normalizedItems"
+            :class="['breadcrumb-item', item.active ? 'active' : null]"
+            @click="onClick(item)"
+            role="presentation">
+            <span v-if="item.active"
+                  v-html="item.text"></span>
             <b-link v-else
                     :to="item.to"
                     :href="item.href || item.link"
-                    v-html="item.text"
-                    @click="onclick"
-            ></b-link>
+                    v-html="item.text"></b-link>
         </li>
         <slot></slot>
     </ol>
@@ -21,23 +20,33 @@
     import bLink from './link.vue';
 
     export default {
-        components: {bLink},
+        components: { bLink },
         computed: {
             componentType() {
                 return this.to ? 'router-link' : 'a';
             },
-            items2() {
-                const last = this.items.length > 0 && this.items[this.items.length - 1];
+            normalizedItems() {
+                let userDefinedActive = false;
+                const originalItemsLength = this.items.length;
 
-                return this.items.map(item => {
+                return this.items.map((item, index) => {
+                    // if no active state is defined,
+                    // default to the last item in the array as active
+                    const isLast = index === originalItemsLength - 1;
+
+                    // nothing defined except the text
                     if (typeof item === 'string') {
-                        return {text: item, link: '#', active: item === last};
+                        return { text: item, link: '#', active: isLast };
                     }
 
-                    if (item.active !== true && item.active !== false) {
-                        item.__active = item === last;
-                    } else {
-                        item.__active = item.active;
+                    // don't default the active state if given a boolean value,
+                    // or if a user defined value has already been given
+                    if (item.active !== true && item.active !== false && !userDefinedActive) {
+                        item.active = isLast;
+                    } else if (item.active) {
+                        // here we know we've been given an active value,
+                        // so we won't set a default value
+                        userDefinedActive = true;
                     }
 
                     return item;
@@ -52,7 +61,7 @@
             }
         },
         methods: {
-            onclick(item) {
+            onClick(item) {
                 this.$emit('click', item);
             }
         }

lib/mixins/link.js

@@ -92,12 +92,14 @@ export default {
         linkClick(e) {
             if (!this.disabled) {
                 this.$root.$emit('clicked::link', this);
-                this.$emit('click');
+                this.$emit('click', e);
+            } else {
+                e.stopPropagation();
             }
 
-            if (this.disabled || (!this.isRouterLink && this._href === '#')) {
+            if (!this.isRouterLink && this._href === '#') {
+                // stop scroll-to-top behavior
                 e.preventDefault();
-                e.stopPropagation();
             }
         }
     }