WIP: Extract hoverClass to module (#409)

* move hoverClass to module

* typescript fix

* adding new tests

* add more test

* add throttle tests

* removing empty space

* improve throttle tests

* update throttle test to include predefined threshold

Author: Lukas Oppermann

__tests__/events/hoverClass.test.ts

@@ -1,61 +1,116 @@
 /* global describe,test,expect,beforeEach,CustomEvent */
-import sortable from '../../src/html5sortable'
+import enableHoverClass from '../../src/hoverClass'
+import store from '../../src/store'
 import {mockInnerHTML} from '../helpers'
 
 describe('Testing mouseenter and mouseleave events for hoverClass', () => {
-  let ul, li
+  let ul, li, liSecond
   // setup html
   beforeEach(() => {
     document.body.innerHTML = mockInnerHTML
     ul = document.querySelector('.sortable')
     li = ul.querySelector('li')
+    liSecond = ul.querySelector('.li-second')
+    // setup data
+    store(ul).config = {
+      hoverClass: 'hover-class',
+      items: 'li',
+      throttleTime: 0
+    }
+    // enable module
+    enableHoverClass(ul, true)
   })
 
-  test('should not add class on hover event', () => {
-    // setup sortable with no hoverClass
-    sortable(ul, {
-      items: 'li',
-      hoverClass: false
-    })
+  test('should not add class on hover event when disabled', () => {
+    // disable hover class
+    enableHoverClass(ul, false)
+    store(ul).setConfig('hoverClass', null)
     // get inital class list length
     let classListLength = li.classList.length
     // trigger mouseenter event
-    li.dispatchEvent(new CustomEvent('mouseenter'))
+    li.dispatchEvent(new MouseEvent('mousemove', {
+      bubbles: true,
+      cancelable: true,
+      buttons: 0,
+      target: li
+    }))
     // assert that class list lenght did not change
     expect(li.classList.length).toBe(classListLength)
   })
-
-  test('should correctly add class on hover event', () => {
-    // setup sortable with hoverClass
-    sortable(ul, {
-      'items': 'li',
-      hoverClass: 'sortable-item-over'
-    })
+  
+  test('should not add class on hover event hoverClass is null', () => {
+    // disable hover class
+    enableHoverClass(ul, false)
+    store(ul).setConfig('hoverClass', null)
+    enableHoverClass(ul, true)
+    // get inital class list length
+    let classListLength = li.classList.length
     // trigger mouseenter event
-    li.dispatchEvent(new CustomEvent('mouseenter'))
+    li.dispatchEvent(new MouseEvent('mousemove', {
+      bubbles: true,
+      cancelable: true,
+      buttons: 0,
+      target: li
+    }))
+    // assert that class list lenght did not change
+    expect(li.classList.length).toBe(classListLength)
+  })
+  
+  test('should correctly add class on hover event, and remove on hover other element', () => {
+    expect(li.classList.contains('hover-class')).toBe(false)
+    // trigger mouse event
+    li.dispatchEvent(new MouseEvent('mousemove', {
+      bubbles: true,
+      cancelable: true,
+      buttons: 0,
+      target: li
+    }))
     // assert that class was added
-    expect(li.classList.contains('sortable-item-over')).toBe(true)
-    // trigger mouseleave event
-    li.dispatchEvent(new CustomEvent('mouseleave'))
+    expect(li.classList.contains('hover-class')).toBe(true)
+    
+    // trigger mouseleave events
+    liSecond.dispatchEvent(new MouseEvent('mousemove', {
+      bubbles: true,
+      cancelable: true,
+      buttons: 0,
+      target: liSecond
+    }))
     // assert that class was removed
-    expect(li.classList.contains('sortable-item-over')).toBe(false)
+    expect(liSecond.classList.contains('hover-class')).toBe(true)
+    expect(li.classList.contains('hover-class')).toBe(false)
   })
 
-  test('should correctly add and remove both classes on hover event', () => {
-    // setup sortable with multiple hoverClasses
-    sortable(ul, {
-      'items': 'li',
-      hoverClass: 'sortable-item-over sortable-item-over-second'
-    })
-    // trigger mouseenter event
-    li.dispatchEvent(new CustomEvent('mouseenter'))
-    // assert that class were added
-    expect(li.classList.contains('sortable-item-over')).toBe(true)
-    expect(li.classList.contains('sortable-item-over-second')).toBe(true)
-    // trigger mouseleave event
-    li.dispatchEvent(new CustomEvent('mouseleave'))
+  test('should remove class when leaving sortable', () => {
+    // trigger mouse event
+    li.dispatchEvent(new MouseEvent('mousemove', {
+      bubbles: true,
+      cancelable: true,
+      buttons: 0,
+      target: li
+    }))
+    // assert that class was added
+    expect(li.classList.contains('hover-class')).toBe(true)
+    // trigger mouseleave events
+    ul.dispatchEvent(new MouseEvent('mouseleave', {
+      bubbles: true,
+      cancelable: true,
+      buttons: 0,
+      target: ul
+    }))
     // assert that class was removed
-    expect(li.classList.contains('sortable-item-over')).toBe(false)
-    expect(li.classList.contains('sortable-item-over-second')).toBe(false)
+    expect(li.classList.contains('hover-class')).toBe(false)
+  })
+  
+  test('should not fire when button is pressed', () => {
+    // trigger mouse event
+    li.dispatchEvent(new MouseEvent('mousemove', {
+      bubbles: true,
+      cancelable: true,
+      buttons: 1
+      target: li
+    }))
+    // assert that class was added
+    expect(li.classList.contains('hover-class')).toBe(false)
   })
+  
 })

__tests__/helpers.ts

@@ -6,7 +6,7 @@ export const mockInnerHTML: string = `
     <span class="handle"></span>
       item
   </li>
-  <li>
+  <li class="li-second">
     <span class="another-handle"></span>
     item 2
   </li>

__tests__/throttle.test.ts

@@ -0,0 +1,41 @@
+/* global describe,test,expect */
+/* eslint-env jest */
+import _throttle from '../src/throttle'
+
+describe('Testing throttle', () => {
+  test('throttle should not allow functions to be called multiple times within the timeframe', () => {
+    let value = 0
+    let fn = _throttle(() => {
+      value++
+    })
+    // call function twice immeditatly
+    fn()
+    fn()
+    // assert
+    expect(value).toBe(1)
+  })
+
+  test('throttle should allow functions to be called multiple times after the timeframe', () => {
+    let value = 0
+    let fn = _throttle(() => {
+      value++
+    }, 10)
+    // call function twice immeditatly
+    global.Date.now = jest.fn(() => 1490760656000)
+    fn()
+    global.Date.now = jest.fn(() => 1490760657000)
+    fn()
+    // assert
+    expect(value).toBe(2)
+  })
+
+  test('throttle should fail if no functin is provided', () => {
+    // assert
+    expect(() => { _throttle('test', 10) }).toThrowError('You must provide a function as the first argument for throttle.')
+  })
+
+  test('throttle should fail if threshold is not a number', () => {
+    // assert
+    expect(() => { _throttle(() => { }, '10') }).toThrowError('You must provide a number as the second argument for throttle.')
+  })
+})

docs/index.html

@@ -493,7 +493,8 @@ <h2 class="h4 mt1">Copy items here</h2>
 		sortable('.js-sortable-buttons', {
 			forcePlaceholderSize: true,
 			items: 'li',
-			placeholderClass: 'border border-white mb1'
+			placeholderClass: 'border border-white mb1',
+      hoverClass: 'bg-yellow'
 		});
 		// buttons to add items and reload the list
 		// separately to showcase issue without reload

src/defaultConfiguration.ts

@@ -14,6 +14,7 @@ export default {
   draggingClass: 'sortable-dragging',
   hoverClass: false,
   debounce: 0,
+  throttleTime: 100,
   maxItems: 0,
   itemSerializer: undefined,
   containerSerializer: undefined,

src/hoverClass.ts

@@ -0,0 +1,41 @@
+/* eslint-env browser */
+import store from './store'
+import _filter from './filter'
+import _throttle from './throttle'
+import { addEventListener as _on, removeEventListener as _off } from './eventListener'
+/**
+ * enable or disable hoverClass on mouseenter/leave if container Items
+ * @param {sortable} sortableContainer a valid sortableContainer
+ * @param {boolean} enable enable or disable event
+ */
+// export default (sortableContainer: sortable, enable: boolean) => {
+export default (sortableContainer: sortable, enable: boolean) => {
+  if (typeof store(sortableContainer).getConfig('hoverClass') === 'string') {
+    let hoverClasses = store(sortableContainer).getConfig('hoverClass').split(' ')
+    // add class on hover
+    if (enable === true) {
+      _on(sortableContainer, 'mousemove', _throttle((event) => {
+        // check of no mouse button was pressed when mousemove started == no drag
+        if (event.buttons === 0) {
+          _filter(sortableContainer.children, store(sortableContainer).getConfig('items')).forEach(item => {
+            if (item !== event.target) {
+              item.classList.remove(...hoverClasses)
+            } else {
+              item.classList.add(...hoverClasses)
+            }
+          })
+        }
+      }, store(sortableContainer).getConfig('throttleTime')))
+      // remove class on leave
+      _on(sortableContainer, 'mouseleave', () => {
+        _filter(sortableContainer.children, store(sortableContainer).getConfig('items')).forEach(item => {
+          item.classList.remove(...hoverClasses)
+        })
+      })
+    // remove events
+    } else {
+      _off(sortableContainer, 'mousemove')
+      _off(sortableContainer, 'mouseleave')
+    }
+  }
+}

src/html5sortable.ts

@@ -18,6 +18,7 @@ import setDragImage from './setDragImage'
 import {default as store, stores} from './store'
 import _listsConnected from './isConnected'
 import defaultConfiguration from './defaultConfiguration'
+import enableHoverClass from './hoverClass'
 /*
  * variables global to the plugin
  */
@@ -283,23 +284,8 @@ export default function sortable (sortableElements, options: object|string|undef
     _enableSortable(sortableElement)
     _attr(listItems, 'role', 'option')
     _attr(listItems, 'aria-grabbed', 'false')
-
-    // Mouse over class
-    // TODO - only assign hoverClass if not dragging
-    if (typeof options.hoverClass === 'string') {
-      let hoverClasses = options.hoverClass.split(' ')
-      // add class on hover
-      _on(listItems, 'mouseenter', function (e) {
-        if (!dragging) {
-          e.target.classList.add(...hoverClasses)
-        }
-      })
-      // remove class on leave
-      _on(listItems, 'mouseleave', function (e) {
-        e.target.classList.remove(...hoverClasses)
-      })
-    }
-
+    // enable hover class
+    enableHoverClass(sortableElement, listItems, true)
     /*
      Handle drag events on draggable items
      Handle is set at the sortableElement level as it will bubble up

src/throttle.ts

@@ -0,0 +1,26 @@
+/**
+ * make sure a function is only called once within the given amount of time
+ * @param {Function} fn the function to throttle
+ * @param {number} threshold time limit for throttling
+ */
+// must use function to keep this context
+export default function (fn: Function, threshold: number = 250) {
+  // check function
+  if (typeof fn !== 'function') {
+    throw new Error('You must provide a function as the first argument for throttle.')
+  }
+  // check threshold
+  if (typeof threshold !== 'number') {
+    throw new Error('You must provide a number as the second argument for throttle.')
+  }
+
+  let lastEventTimestamp = null
+
+  return (...args) => {
+    let now = Date.now()
+    if (lastEventTimestamp === null || now - lastEventTimestamp >= threshold) {
+      lastEventTimestamp = now
+      fn.apply(this, args)
+    }
+  }
+}

src/types/configuration.d.ts

@@ -1,15 +1,16 @@
 interface configuration {
-  connectWith: boolean,
-  acceptFrom: void,
-  copy: boolean,
-  placeholder: void,
-  disableIEFix: boolean,
-  placeholderClass: string,
-  draggingClass: string,
-  hoverClass: boolean,
-  debounce: number,
-  maxItems: number,
-  itemSerializer: void,
-  containerSerializer: void,
-  items: string
+  connectWith?: string,
+  acceptFrom?: void,
+  copy?: boolean,
+  placeholder?: void,
+  disableIEFix?: boolean,
+  placeholderClass?: string,
+  draggingClass?: string,
+  hoverClass?: string,
+  debounce?: number,
+  throttleTime?: number,
+  maxItems?: number,
+  itemSerializer?: void,
+  containerSerializer?: void,
+  items?: string
 }

tsconfig.json

@@ -1,8 +1,7 @@
 {
   "compilerOptions": {
     "module": "commonjs",
-    "target": "ESNext",
-    "strictNullChecks": true
+    "target": "ESNext"
   },
   "typeRoots": [
     "./src/types",