Is there an alternative possibly cleaner way (without so much repetition) to write the following? I'm thinking about the headache it's going to be when I have to remove all of my even listers.
customCursor() {
const prevButton = document.querySelector('.previous');
const nextButton = document.querySelector('.next');
const brandLogo = document.querySelector('.brand-logo');
const navButton = document.querySelector('.nav-button');
const carouselCellLink = document.querySelectorAll('.carousel-cell__link');
const cursorContainer = this.cursorContainer;
const cursorOffset = {
left : 50,
top : 50
}
document.querySelector('.page-flickity').addEventListener("mousemove", function (e) {
cursorContainer.style.left = (e.pageX - cursorOffset.left) + 'px';
cursorContainer.style.top = (e.pageY - cursorOffset.top) + 'px';
}, false);
prevButton.addEventListener('mouseover', () => {
cursorContainer.classList.add('previous');
});
prevButton.addEventListener('mouseleave', () => {
cursorContainer.classList.remove('previous');
});
nextButton.addEventListener('mouseover', () => {
cursorContainer.classList.add('next');
});
nextButton.addEventListener('mouseleave', () => {
cursorContainer.classList.remove('next');
});
brandLogo.addEventListener('mouseover', () => {
cursorContainer.classList.add('hidden');
});
brandLogo.addEventListener('mouseleave', () => {
cursorContainer.classList.remove('hidden');
});
navButton.addEventListener('mouseover', () => {
cursorContainer.classList.add('hidden');
});
navButton.addEventListener('mouseleave', () => {
cursorContainer.classList.remove('hidden');
});
for(var i = 0; i < carouselCellLink.length; i++) {
carouselCellLink[i].addEventListener('mouseover', () => {
cursorContainer.classList.add('link');
});
carouselCellLink[i].addEventListener('mouseleave', () => {
cursorContainer.classList.remove('link');
});
}
}
-
1\$\begingroup\$ Tell us more about what you are really trying to achieve with these listeners. What are the CSS classes for? A live demo would be helpful — you can make one by pressing Ctrl-M in the question editor. \$\endgroup\$200_success– 200_success2016年11月02日 19:59:23 +00:00Commented Nov 2, 2016 at 19:59
-
\$\begingroup\$ Not familiar with Javascript but you could use an array of objects and create button in a for loop and set its listener in the same for loop. \$\endgroup\$thepace– thepace2016年11月04日 10:52:15 +00:00Commented Nov 4, 2016 at 10:52
2 Answers 2
Somewhat obvious rule: the good way to avoid repetition is always to factorize as much as possible.
Here this can be achieved for all but page-flickity
of your targets, which are almost identical: only involved elements and added/removed class-names vary.
It already reduces the code to add listeners, like this:
function customCursor() {
const
todo = {
'previous': [false, 'previous'],
'next': [false, 'next'],
'brand-logo': [false, 'hidden'],
'nav-button': [false, 'hidden'],
'carousel-cell__link': [true, 'link']
},
events = {'mouseover': true, 'mouseleave': false},
cursorContainer = this.cursorContainer,
cursorOffset = {left : 50, top : 50};
document.querySelector('.page-flickity').addEventListener(
'mousemove',
function (e) {
cursorContainer.style.left = (e.pageX - cursorOffset.left) + 'px';
cursorContainer.style.top = (e.pageY - cursorOffset.top) + 'px';
},
false
);
var className, action, elements, i, n;
for (className in todo) {
action = todo[className];
elements = action[0]
? document.querySelectorAll(className)
: [document.querySelector(className)];
for (i = 0, n = elements.length; i < n; i++) {
for (var event in events) {
elements[i].addEventListener(
event,
() => cursorContainer.classList.toggle(action[1], events[event])
)
}
}
}
}
The first important point is the todo
object, where we register all targets:
- each key is the class-name to operate
querySelector[All]
on - the corresponding value states true/false to indicate if all elements must be selected or not, and the class-name which will be added/removed when the mouse events occur
Based on that we iterate this object, then for each target we iterate the collection of involved elements (note that the unique element returned by querySelector
is turned into an array).
Then it becomes pretty easy to adapt it for also removing them.
This is achieved by introducing a "listeners archive", populated when adding listeners, then returned by the function:
function customCursor(listeners) {
const
todo = {
'previous': [false, 'previous'],
'next': [false, 'next'],
'brand-logo': [false, 'hidden'],
'nav-button': [false, 'hidden'],
'carousel-cell__link': [true, 'link']
},
events = {
'mouseover': true,
'mouseleave': false
},
cursorContainer = this.cursorContainer,
cursorOffset = {left : 50, top : 50};
document.querySelector('.page-flickity').addEventListener(
'mousemove',
function (e) {
cursorContainer.style.left = (e.pageX - cursorOffset.left) + 'px';
cursorContainer.style.top = (e.pageY - cursorOffset.top) + 'px';
},
false
);
addListeners = !listeners;
listeners = listeners || [];
var className, action, elements, element, i, n;
if (addListeners) {
for (className in todo) {
action = todo[className];
elements = action[0]
? document.querySelectorAll(className)
: [document.querySelector(className)];
for (i = 0, n = elements.length; i < n; i++) {
element = elements[i];
for (var event in events) {
listener = function() {
cursorContainer.classList.toggle(action[1], events[event])
};
element.addEventListener(event, listener);
listeners.push([element, event, listener]);
}
}
}
return listeners;
} else {
for (i in listeners) {
listener = listeners[i];
listener[0].removeEventListener(listener[1], listener[2]);
}
}
}
(please note: I didn't do anything about the "lonely" event about page-flickity
element; you may do your own here)
Then it's up to you to pass this archive in to the function when you want to remove these listeners:
// add listeners:
var listeners = customCursor();
// remove listeners:
customCursor(listeners);
My primary concern is over how you are coupling this functionality to the DOM. Do you ever foresee a use case where more than one of these controls might exist on a page? If so, your approach of binding functionality to the first arbitrary element instance of a class in the DOM will break. Every control you might want to apply to the page would be bound to this first set of elements of every class.
Should you really be using document.querySelector()
against class names or would it be better to enforce more specific behavior by operating against specific elements based on id and using getElementById()
?
If you truly wanted to introduce the flexibility of query-based selection, then perhaps you might be better off to utilize querySelectorAll()
and act against the collection of elements that are selected rather than always only picking the first element from DOM that meets the selector.
As far as attaching event listeners, I would consider building a configuration object that could be passed to customCursor()
to pass along configuration information and callbacks. This begins to allow you to decouple the callback actions (adding/removing classes) from being hard coded in the customCursor()
definition.
Perhaps that is something like this:
var cursorConfig =
{
prev: {
el: document.getElementById('prev'),
callbacks: {
'mouseover': function() { ... },
// or call some common function defined elsewhere
'mouseout': cursorMouseout(),
...
}
}
next: { ... },
...
}
You could easily take this object and iterate it to attach event handlers programmatically. That might look something like:
// assume we are in customCursor() with config holding this config object
for(key in config) {
var el = config[key].el;
for (event in config[key].callbacks) {
return el.addEventListener(event, config[key].callbacks[event]);
}
}
By taking the approach of specifically binding behaviors to unique DOM elements and by having config-driven instantiation of an individual cursor, you could foresee usage like this where you can create multiple control elements on a page.
// set up one cursor
// maybe this first one is for carousel like your example
var carouselCursorConfig = { ... };
var carouselCursor = customCursor(carouselCursorConfig);
// maybe the second control performs category navigation
var categoryCursorConfig = { ... };
var categoryCursor = customCursor(categoryCursorConfig);
Indent your code inside your function.