Open noeldelgado opened 8 years ago
@richvdh can you please review this? I am asking to make sure this change does not break something on your side.
I just checked and can confirm that on this case if (DONT_CREATE_GEMINI && this.onResize)
we don't need to move nodes, create, neither add class names to elements, the scrolling will still work as expected and the onResize
callback will still be called.. in fact this was the line that makes me notice the issue 106: addClass(this.element, [CLASSNAMES.element]);
because that one is preventing the native scroll behaviour (when “overlay-scrollbars” are supported natively and an onResize
callback is passed).
@noeldelgado: it's hard for me to test this, because my OS doesn't support overlay-scrollbars, but my thinking is as follows:
onResize
is supposed to be called when the external size (offsetWidth
/offsetHeight
) of the div changes. However, if you put the resize-trigger inside the div, then it will match the internal size (scrollWidth
/scrollHeight
) of the div. I think you won't see this on your example, because the internal and external size of #example-7
are always the same.
So even if the OS supports native scrollbars, you need an outer div which doesn't scroll, which contains both the resize-trigger and the viewElement
which does the scrolling.
I guess the inner viewElement
could use the native scroll behaviour if DONT_CREATE_GEMINI
is false. I think I made it use the gemini scrollbar in this case because it was easier for me to get right without being able to test on OS X.
(In fact, as things are here, the resize-trigger won't match the size at all, because the width==height==100%
trick only works if the parent has position: relative
, but that is probably solved more easily.)
Thank you @richvdh, I’ve been thinking this for a while today and this is what I’m planning to do:
v1.x.x
on a new branch (v1
for instance) for further additionsv2.0.0
This way we can have both versions and see how it goes, as you may know npm
allows us to publish previous versions, so updating v1/v2 shouldn’t be a problem.
Also, users can choose which version fits better for their needs (the auto-update-call or calling the update
method manually).
I like the idea of having the resize-trigger
and the onResize
hook, that solves some specific use-cases, but being honest I haven’t really needed them yet, the update
method was designed to be called on demand so my programs followed that approach and after upgrading I noticed it ends up calling the update
method several times (because I am calling it manually too), so I need to update all my programs if I upgrade them to use the latest version of gemini.
That’s basically why I am considering on keeping both versions.
What do you think?
Well, it's up to you, of course, but personally, I would be wary about ending up maintaining two branches of the code. Also, most people won't take the time to check the different versions of the project and will always use the "latest".
If you're proposing to make the resize-trigger optional, then I think it would make more sense to make it configurable with a parameter to the constructor.
When native scrollbars are supported and an
onResize
callback is passed, we need to create the resize trigger object for the event to be fired but we do not need to create a separeted_viewElement
.The way gemini works is that it will use the native scrollbars if the OS supports “overlay-scrollbars” (which means that the
element
you pass on the configuration should be scrollable by defaultoverflow: auto|scroll
).