Scroll Event Listener no longer works with 4.0.0-alpha.4 and above

[roskelld]

Current Behavior

Scroll Event Listener attached to the DOM element no longer fire in versions 4.0.0-alpha.4 and up.

Expected behavior

On releases versions 4.0.0-alpha.3 and prior the following would produce a console out put every time the content panel scrolled.

simplebarcontent.addEventListener( 'scroll', () => console.log('Scrolling'), false );

Reproducible example

Here's an example which should replace the text in the p with the timestamp of the scroll to show the event listener. In older versions this works, but not with the new alphas
https://jsfiddle.net/Lqzoa8cm/

Your environment

Software	Version(s)
SimpleBar	4.0.0-alpha.4 and up
Browser	Firefox 66.0.3 & Chrome 73.0.3683.103
npm/Yarn
Operating System	Windows 10

[andersk]

You need to add the event listener to content.SimpleBar.getScrollElement(), not to content itself—see the documentation.

I can’t reproduce your claim that your version worked in 4.0.0-alpha.3; AFAIK it never has.

[roskelld]

Thanks @andersk that's good to know. I literally have a setup on my desktop using the same code as posted and toggling between 4.0.0-alpha.3 and 4.0.0-alpha.4 has it working, then not working, so perhaps I've been using a bad setup that somehow worked for a time.

I'll update my stuff to point to content.SimpleBar.getScrollElement() and test.

[roskelld]

Ok, can confirm the documented method works (whodathunkit?). I'll close this.

Personal lesson for Friday: Read Documentation.

[andersk]

Your fiddle with 4.0.0-alpha.6 directly replaced by 4.0.0-alpha.3:
https://jsfiddle.net/anderskaseorg/mx85u1n2/
Doesn’t work for me in Firefox 66 or Chrome 75.

[Grsmto]

Hey, yeah unfortunately that was a breaking change of v4.
Might have to display a disclaimer on the README about the changelog.

[andersk]

The code in this report didn’t work in v3 either:
https://jsfiddle.net/anderskaseorg/m5aw2hbo/
There were breaking changes in v4, but this isn’t one of them.

[roskelld]

const el = new SimpleBar(document.getElementById('myElement'));
el.getScrollElement().addEventListener('scroll', function(...));

I just noticed that the documentation states el.getScrollElement() but @andersk comment above was content.SimpleBar.getScrollElement() I actually found that @andersk solution worked, but not the documented one. My code needed the SimpleBar portion to add the eventlistener.

[andersk]

In the documentation, “el” refers to the SimpleBar object, not the HTMLElement object. You can get the SimpleBar object either from the new SimpleBar(…) constructor or afterwards from the .SimpleBar attribute of the HTMLElement object.

Although it’s true that the .SimpleBar attribute isn’t documented. I learned about it by reading the source. Is it considered part of the public interface?

[Grsmto]

That's totally a mistake in the doc. I'll correct that right now. Thanks!

[Grsmto]

Ah no @andersk is right :D that's very confusing tho!

[Grsmto]

So no, the idea of the public interface was:

const simpleBar = new SimpleBar(document.getElementById('myElement'));
simpleBar.getScrollElement().addEventListener('scroll', function(...));

The HTMLElement.SimpleBar was actually just a flag to know if an element was already instantiated or not.

[roskelld]

Yeah, in my setup I've been making a reference to the HTMLElement then creating the SimpleBar Object, rather than referencing the Object directly.
This is good to grasp as I missed the detail in this, probably partly driven by me adding SimpleBar to my existing code base.

[andersk]

@Grsmto There’s no way to do that with a declaratively instantiated SimpleBar (<div data-simplebar>), right? I think we want some supported way to call getScrollElement and getContentElement given only an HTMLElement. I’ve found .SimpleBar convenient, but maybe a static method would be more future-proof (SimpleBar.ofElement(…)?).

[Grsmto]

@andersk no there is no "official" way of doing that but yeah I have been doing with the .simplebar reference and it's been working fine.
We could definitely also have a static method but I don't know if it would be more future-proof since I don't see the .simplebar ever disappear.
I think we could just document that actually. What do you think?

But yes that needs documentation, especially since that's the "recommended" way of instantiating SimpleBar.

[andersk]

I think most of the arguments about why we might want to avoid custom DOM properties are presented here, and an argument I might add is better compatibility with static type systems. The cleanest alternative to the .SimpleBar property would be to maintain a WeakMap from HTMLElement to SimpleBar. That said, I don’t think I care too much one way or the other.