-
Notifications
You must be signed in to change notification settings - Fork 2
STIJ-196: Added JS namespaces setup. #171
base: develop
Are you sure you want to change the base?
Changes from 1 commit
e9e5ba1
ddfa152
ac6f860
4db71d6
ceecec3
5063fac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,12 @@ | ||
var gent_styleguide = (function () { // eslint-disable-line no-unused-vars | ||
var gent_styleguide = gent_styleguide || {}; | ||
|
||
var base = gent_styleguide.base = (function () { // eslint-disable-line no-unused-vars | ||
'use strict'; | ||
|
||
/** | ||
* Generates a tabTrap object | ||
* Generates a tabTrap object. | ||
* | ||
* @param {object} container DOM-element | ||
* @param {object} container DOM-element. | ||
* @constructor | ||
*/ | ||
function TabTrap(container) { | ||
|
@@ -60,8 +62,44 @@ var gent_styleguide = (function () { // eslint-disable-line no-unused-vars | |
this.hasFocusables = focusables && focusables.length > 0; | ||
} | ||
|
||
/** | ||
* Generates a Helper object. | ||
* | ||
* @constructor | ||
*/ | ||
function Helper() { | ||
|
||
/** | ||
* Removes a class from a DOM-element. | ||
* | ||
* @param {object} element DOM-element. | ||
* @param [string] className A class name. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My phpstorm doesn't understand this bracket notation. |
||
*/ | ||
this.removeClass = function (element, className) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice idea, but are these really helpful? We could potentially use them as a polyfill for IE9, using string.split(' ') on element.className. |
||
if (element.classList) { | ||
element.classList.remove(className); | ||
} | ||
}; | ||
|
||
/** | ||
* Adds a class from a DOM-element. | ||
* | ||
* @param {object} element DOM-element. | ||
* @param [string] className A class name. | ||
*/ | ||
this.addClass = function (element, className) { | ||
if (element.classList) { | ||
element.classList.add(className); | ||
} | ||
else { | ||
element.classList += ' ' + className; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Element.classList is falsy. |
||
} | ||
}; | ||
} | ||
|
||
return { | ||
TabTrap: TabTrap | ||
TabTrap: TabTrap, | ||
Helper: Helper | ||
}; | ||
|
||
})(); | ||
})(base); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,15 +2,16 @@ | |
* @file | ||
* Javascript binding of hamburger-menu.functions.js. | ||
*/ | ||
(function ($) { | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Place scripts at the bottom of the page
Also: if you insist on placing them in 'head', use DOMContentLoaded. |
||
window.onload = (function () { | ||
'use strict'; | ||
|
||
$(window).on('load', function (e) { | ||
var $hamburgerMenu = $('.hamburger-menu'); | ||
return function () { | ||
var hamburgerMenu = document.querySelectorAll('.hamburger-menu'); | ||
|
||
if ($hamburgerMenu.length > 0) { | ||
$hamburgerMenu.loadHamburgerMenu(); | ||
for (var i = hamburgerMenu.length; i--;) { | ||
var menu = new hamburger_component.HamburgerMenu(hamburgerMenu[i]); // eslint-disable-line no-undef | ||
menu.loadHamburgerMenu(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the idea of the HamburgerMenu returning functions, but I would load the menu automatically. |
||
} | ||
}); | ||
|
||
})(jQuery); | ||
}; | ||
})(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,36 +1,29 @@ | ||
/** | ||
* @file | ||
* Implements a hamburger-menu button combined with | ||
* a slide-in panel for easy mobile navigation. | ||
* | ||
* @authors | ||
* Wim Vantomme | ||
* Bart Delrue | ||
* | ||
*/ | ||
(function ($) { | ||
var gent_styleguide = gent_styleguide || {}; | ||
|
||
var hamburger_component = gent_styleguide.components = (function (context) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now you can't extend gent_styleguide.components anymore. I'd use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or omit 'components' altogether, but that would be a structural decision. |
||
'use strict'; | ||
|
||
$.fn.extend({ | ||
|
||
/** | ||
* Creates a jQuery extension function. | ||
* | ||
* @fires event:click | ||
*/ | ||
loadHamburgerMenu: function () { | ||
var $drawer = $(this[0]).find('.hamburger-menu__drawer'); | ||
var $closeBtn = $drawer.find('.close'); | ||
var $overlay = $('.hamburger-menu__overlay'); | ||
/** | ||
* Creates a hamburger menu object. | ||
* | ||
* @param {object} element DOM-element. | ||
* @constructor | ||
*/ | ||
function HamburgerMenu(element) { | ||
this.loadHamburgerMenu = function () { | ||
|
||
var drawer = element.querySelector('.hamburger-menu__drawer'); | ||
var closeBtn = drawer.querySelector('.close'); | ||
var overlay = element.querySelector('.hamburger-menu__overlay'); | ||
var trigger; | ||
|
||
if (typeof gent_styleguide === 'undefined') { | ||
console.error('You need to include base.js.'); // eslint-disable-line no-console | ||
return; | ||
} | ||
|
||
// TabTrap doens't use jquery opbjects. | ||
var tabTrap = new gent_styleguide.TabTrap($drawer[0]); // eslint-disable-line no-undef | ||
var helper = new base.Helper(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to create a new instance of 'Helper'. |
||
var tabTrap = new base.TabTrap(drawer); // eslint-disable-line no-undef | ||
|
||
/** | ||
* Closes the hamburger menu | ||
|
@@ -41,8 +34,9 @@ | |
if (e) { | ||
e.preventDefault(); | ||
} | ||
$drawer.removeClass('js-opened'); | ||
$overlay.removeClass('js-opened'); | ||
helper.removeClass(drawer, 'js-opened'); | ||
helper.removeClass(overlay, 'js-opened'); | ||
|
||
document.removeEventListener('keydown', handleKeyboardInput); | ||
tabTrap.reset(); | ||
|
||
|
@@ -55,7 +49,7 @@ | |
// remove the menu from the tabindex | ||
// jquery .css() doesn't now 'important' | ||
setTimeout(function () { | ||
$drawer.attr('style', 'display: none'); | ||
drawer.setAttribute('style', 'display: none'); | ||
}, 500); | ||
}; | ||
|
||
|
@@ -71,11 +65,11 @@ | |
|
||
// add the menu to the tabindex | ||
// jquery .css() doesn't now 'important' | ||
$drawer.attr('style', 'display: block'); | ||
drawer.setAttribute('style', 'display: block'); | ||
|
||
setTimeout(function () { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I should have commented this, took me a moment to figure out why we have to wait for the next tick. |
||
$drawer.addClass('js-opened'); | ||
$overlay.addClass('js-opened'); | ||
helper.addClass(drawer, 'js-opened'); | ||
helper.addClass(overlay, 'js-opened'); | ||
}); | ||
|
||
|
||
|
@@ -84,7 +78,7 @@ | |
trigger.setAttribute('aria-expanded', true); | ||
|
||
// set focus to the menu | ||
$drawer.focus(); | ||
drawer.focus(); | ||
|
||
// handle keyboard input | ||
document.addEventListener('keydown', handleKeyboardInput); | ||
|
@@ -144,8 +138,10 @@ | |
* | ||
* @event click | ||
*/ | ||
$('.hamburger-menu__toggle').on('click', open); | ||
|
||
var toggle = element.querySelector('.hamburger-menu__toggle'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. define variables on top |
||
if (toggle) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the check. But
Suggestion: |
||
toggle.addEventListener('click', open); | ||
} | ||
|
||
/** | ||
* Indicates that a user has clicked on the closeBtn hamburger menu | ||
|
@@ -155,10 +151,17 @@ | |
* | ||
* @event click | ||
*/ | ||
$closeBtn.add($overlay).on('click', close); | ||
closeBtn.addEventListener('click', close); | ||
overlay.addEventListener('click', close); | ||
|
||
// init the menu as closed on startup | ||
close(); | ||
} | ||
}); | ||
})(jQuery); | ||
|
||
}; | ||
} | ||
|
||
return { | ||
HamburgerMenu: HamburgerMenu | ||
}; | ||
|
||
})(hamburger_component); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary variable.
Since gent_styleguide is already an object, we can just append properties to it.
gent_styleguide.tabtrap = ...
gent_styleguide.helper = ...