0
\$\begingroup\$

I'm working on a website where different things can occur on resize and scroll events. I only want one event handler that comes into play, whenever something in the DOM must be changed.

So I created a TaskManager where you can register objects and callbacks, that are run, when the specific event is fired.

The TaskManager is injected into other objects, where this.taskManager.registerTask() is called to send all necessary information.

The TaskManager then executes all registered tasks, as soon as an event occurs. It also sends the current scrollTop-value to the callback.

class TaskManager {
  constructor() {
    this.DOM = {
      html: document.getElementsByTagName('html')[0],
    }

    this.tasks = {
      scroll: [],
      resize: [],
    };

    window.addEventListener('scroll', event => {
      window.requestAnimationFrame(timestamp => {
        this.processTasksOnScroll();
      });
    });
  }

  registerTask(type, task) {
    if (!this.tasks[type]) {
      return false;
    }

    this.tasks[type].push(task);
    return true;
  }

  processTasksOnScroll() {
    const scrollTop = window.scrollY || window.scrollTop || this.DOM.html.scrollTop;

    for (const task of this.tasks.scroll) {
      task.target[task.callback](scrollTop);
    }
  }
}

class Test {
  constructor(config) {
    this.taskManager = config.taskManager || null;

    if (this.taskManager) {
        this.taskManager.registerTask('scroll', {target: this, callback: 'onScroll'});
    }
  }

  onScroll(scrollTop) {
    console.log('Callback in Test with scrollTop: ' + scrollTop);
  }
}

const taskManager = new TaskManager();
const test = new Test({taskManager: taskManager});
.test {
  height: 120vh;
  padding: 20px;
  background: rgba(0, 0, 0, 0.2);
  text-align: center;
}
<div class="test">Scroll here …</div>

Is this a good approach or could something be improved?

\$\endgroup\$
  • \$\begingroup\$ I'm having difficulty understanding why you'd do this over just doing window.addEventListener('scroll', myFunc). You said that you only want one event handler, but this isn't much different from what the browser does natively. Besides, you don't have resize working in this code -- are you planning to manually create a light proxy for every event type? \$\endgroup\$ – Kevin Raoofi Aug 8 '18 at 15:38
  • \$\begingroup\$ @KevinRaoofi Thanks a lot for your comment. I thought it might be problematic if I have window.addEventListener('scroll', event => {}); on multiple of these objects - like having multiple click-handler on the same button, which might get confusing at some point. I thought it might be helpful to have an overview of all functions that are called on scroll. But if you say, that it doesn't matter how many times window.addEventListener('scroll', event => {}); is set, then I can refrain from this solution and add it directly to the objects that need it. \$\endgroup\$ – lampshade Aug 8 '18 at 16:36
  • 1
    \$\begingroup\$ It doesn't matter how many times you add an event listener to something. Of course, there would be overhead, but it's more or less equivalent to the overhead in your code except now it's non-standard. \$\endgroup\$ – Kevin Raoofi Aug 8 '18 at 16:50
3
\$\begingroup\$

Some points.

  • window is the default object. You don't need to use is. It is highlighted by the fact you don't use it sometimes window.document, or window.TaskManager but then for objects you are unsure of window.addEventListener you add it.

  • It is not a good idea to stack requestAnimationFrame requests.

    1. Events related to mouse movements can fire at rates much higher than the display refresh rate of 60fps.

    2. It introduces an up to 16ms delay on the event (and yes users will notice it as a sluggish response).

    3. If used in conjunction with standard event handlers you will be handling events out of order.

    4. Completely decouples the handler from the event.

  • You have decoupled (ignoring above point) the event from the handler because you don't pass on the event object. If you are going to manage events you should pass on the event object so that the handler can access the event.

  • registerTask will happily push events to the stack without checking if the supplied object is a valid. When you come to handling the event eg processTasksOnScroll it will throw if task.target or task.callback do not result in a function reference. If it throws none of the following events will be handled.

  • If any of the tasks throws an uncaught error then all following tasks will not be called.

Without knowing more about the project its hard to say more about this approch.

I do feel personally that events should not be one to many. For each event object created there should be one unbroken execution path that completely handles the event. Having events trigger more events is just going to result in an unmanageable mess, as you add code to conditional communicate between handlers wanting priority, or exclusivity.

That is not to say you can not have one handler for many events and then selectively redirect the event to a specific handler.

\$\endgroup\$
  • \$\begingroup\$ Regarding requestAniamtionFrame. MDN says it's a good thing to throttle resize- and scroll-events. That's why I followed their examples - and yes, I see that I forgot to add the running-parameter - oh boy. However, do you think this is still needed at all? \$\endgroup\$ – lampshade Aug 8 '18 at 16:44
  • \$\begingroup\$ My main concern is, that I end up attaching window.addEventListener('scroll', event => {}); to multiple objects. If this is something, not to worry about, I will refrain from this solution. I thought it might be problematic, if many different places attach their own event handler for global things like scroll or resize. \$\endgroup\$ – lampshade Aug 8 '18 at 16:47
  • \$\begingroup\$ @lampshade when throttling you have one requestAnimationFrame (rAF) loop , and the events just save the current state. rAF then updates according to the most recent state. You don't send all the event to rAF, that's not throttling its just delaying. Without more details on the project I can not give much more info. \$\endgroup\$ – Blindman67 Aug 8 '18 at 16:49
  • 1
    \$\begingroup\$ @lampshade You can have one event handler that passes the event to associated handler, But dont pass it all to all. \$\endgroup\$ – Blindman67 Aug 8 '18 at 16:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.