From 48ec70250cd63f5558f30698723555656eceff64 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 1 Aug 2017 11:55:37 +0100 Subject: [PATCH] reverted the JS timeout code improved specs so that they pass --- app/assets/javascripts/fly_out_nav.js | 52 ++++--------------------- app/assets/stylesheets/new_sidebar.scss | 6 +-- spec/javascripts/fly_out_nav_spec.js | 31 +++------------ 3 files changed, 16 insertions(+), 73 deletions(-) diff --git a/app/assets/javascripts/fly_out_nav.js b/app/assets/javascripts/fly_out_nav.js index 93101f123b5..67a58493fae 100644 --- a/app/assets/javascripts/fly_out_nav.js +++ b/app/assets/javascripts/fly_out_nav.js @@ -1,16 +1,3 @@ -let hideTimeoutInterval = 0; -let hideTimeout; -let subitems; - -export const getHideTimeoutInterval = () => hideTimeoutInterval; - -export const hideAllSubItems = () => { - subitems.forEach((el) => { - el.parentNode.classList.remove('is-over'); - el.style.display = 'none'; // eslint-disable-line no-param-reassign - }); -}; - export const calculateTop = (boundingRect, outerHeight) => { const windowHeight = window.innerHeight; const bottomOverflow = windowHeight - (boundingRect.top + outerHeight); @@ -24,14 +11,8 @@ export const showSubLevelItems = (el) => { if (!$subitems) return; - hideAllSubItems(); - - if (el.classList.contains('is-over')) { - clearTimeout(hideTimeout); - } else { - $subitems.style.display = 'block'; - el.classList.add('is-over'); - } + $subitems.style.display = 'block'; + el.classList.add('is-over'); const boundingRect = el.getBoundingClientRect(); const top = calculateTop(boundingRect, $subitems.offsetHeight); @@ -46,37 +27,20 @@ export const showSubLevelItems = (el) => { export const hideSubLevelItems = (el) => { const $subitems = el.querySelector('.sidebar-sub-level-items'); - const hideFn = () => { - el.classList.remove('is-over'); - $subitems.style.display = 'none'; - $subitems.classList.remove('is-above'); - hideTimeoutInterval = 0; - }; - - if ($subitems && hideTimeoutInterval) { - hideTimeout = setTimeout(hideFn, hideTimeoutInterval); - } else if ($subitems) { - hideFn(); - } -}; + if (!$subitems) return; -export const setMouseOutTimeout = (el) => { - if (el.closest('.sidebar-sub-level-items')) { - hideTimeoutInterval = 250; - } else { - hideTimeoutInterval = 0; - } + el.classList.remove('is-over'); + $subitems.style.display = 'none'; + $subitems.classList.remove('is-above'); }; export default () => { - const items = [...document.querySelectorAll('.sidebar-top-level-items > li:not(.active)')]; - subitems = [...document.querySelectorAll('.sidebar-top-level-items > li:not(.active) .sidebar-sub-level-items')]; + const items = [...document.querySelectorAll('.sidebar-top-level-items > li:not(.active)')] + .filter(el => el.querySelector('.sidebar-sub-level-items')); items.forEach((el) => { el.addEventListener('mouseenter', e => showSubLevelItems(e.currentTarget)); el.addEventListener('mouseleave', e => hideSubLevelItems(e.currentTarget)); }); - - subitems.forEach(el => el.addEventListener('mouseleave', e => setMouseOutTimeout(e.target))); }; diff --git a/app/assets/stylesheets/new_sidebar.scss b/app/assets/stylesheets/new_sidebar.scss index 72c12413aba..88486a12379 100644 --- a/app/assets/stylesheets/new_sidebar.scss +++ b/app/assets/stylesheets/new_sidebar.scss @@ -226,10 +226,10 @@ $new-sidebar-width: 220px; &::before { content: ""; position: absolute; - top: -20px; - bottom: -20px; + top: -30px; + bottom: -30px; left: 0; - right: -20px; + right: -30px; z-index: -1; } diff --git a/spec/javascripts/fly_out_nav_spec.js b/spec/javascripts/fly_out_nav_spec.js index 0fdaa2d8663..61e6c9f1fdb 100644 --- a/spec/javascripts/fly_out_nav_spec.js +++ b/spec/javascripts/fly_out_nav_spec.js @@ -1,7 +1,5 @@ import { calculateTop, - setMouseOutTimeout, - getHideTimeoutInterval, hideSubLevelItems, showSubLevelItems, } from '~/fly_out_nav'; @@ -41,26 +39,6 @@ describe('Fly out sidebar navigation', () => { }); }); - describe('setMouseOutTimeout', () => { - it('sets hideTimeoutInterval to 150 when inside sub items', () => { - el.innerHTML = ''; - - setMouseOutTimeout(el.querySelector('.js-test')); - - expect( - getHideTimeoutInterval(), - ).toBe(150); - }); - - it('resets hideTimeoutInterval when not inside sub items', () => { - setMouseOutTimeout(el); - - expect( - getHideTimeoutInterval(), - ).toBe(0); - }); - }); - describe('hideSubLevelItems', () => { beforeEach(() => { el.innerHTML = ''; @@ -142,16 +120,17 @@ describe('Fly out sidebar navigation', () => { it('sets is-above when element is above', () => { const subItems = el.querySelector('.sidebar-sub-level-items'); - subItems.style.height = '5000px'; + subItems.style.height = `${window.innerHeight + el.offsetHeight}px`; + subItems.style.position = 'absolute'; el.style.position = 'relative'; - el.style.top = '1000px'; + el.style.top = `${window.innerHeight - el.offsetHeight}px`; - spyOn(el.classList, 'add'); + spyOn(subItems.classList, 'add'); showSubLevelItems(el); expect( - el.classList.add, + subItems.classList.add, ).toHaveBeenCalledWith('is-above'); }); }); -- GitLab