Highlight the importance of using "onclick" in README

[lxender]

While toying around with the examples given in the README, I found out that rewriting onclick with the equivalent syntax for addEventListener breaks the component. I think it's important to explicitly mention this requirement below the example in README.md, although supporting addEventListener probably is a future goal.

[valdrinkoshi]

How does the component break? You can safely add the click listener with child.addEventListener('click', callback), just make sure you don't redefine the callback each time

[lxender]

Unlike using onclick, child.addEventListener('click', () => console.log(`clicked item #${index}`)) still listens for the removed element, resulting in multiple callbacks for one click. You'd need to remove it manually inside scroller.recycleElement, like I do here:

function clickHandler(index) {
    console.log(`clicked item #${index}`)
}

scroller.updateElement = (child, item, index) => {
    child.textContent = index + ' - ' + item;
    child.addEventListener("click", () => clickHandler(index));
};

scroller.recycleElement = (item) => {
    item.removeEventListener("click", () => clickHandler());
}

It's unexpected for me, but it might just be a feature of that function I wasn't aware of.

[valdrinkoshi]

The example you provided won't work, as you're creating a new function both when adding & removing the event listener.

You can add the same instance of your function instead of creating a new one, e.g.

function clickHandler(event) {
    console.log(`clicked item #${event.target.index}`)
}

scroller.updateElement = (child, item, index) => {
    child.textContent = index + ' - ' + item;
    child.index = index;
    child.addEventListener("click", clickHandler);
};

In the README example we're using onclick setter as it's more contrived and allows to pass information like item, index into the function. There are many other ways to pass around this information, e.g. store it as properties/attributes on the child element. Do you think adding the above example would help clarify?

[lxender]

Oh, I didn't consider storing it on child and I didn't see how directly passing the callback was like redefining it 😅 Thanks for clarifying!

I think using addEventListener instead of the on... equivalent is taught as a best practice nowadays. Therefore, it's probably good to add your code as an example usage.

[domenic]

Closed in https://github.com/valdrinkoshi/virtual-scroller/commit/fb8f6fc65cd7e89c135b3ab92a3492e3ef41d66c, after a bit of a delay....