activate panel on scroll even when snapping is disabled

[akreitals]

Panel activate event and any corresponding menu's now respond to scrolling even when snapping is disabled.

You can see an example using my fork in this Plunk.

We may want to play with some kind of additional threshold here as well.

[guidobouman]

I'm not sure yet if I want this behaviour to actually take place. If you disable the plugin but the menu still works, is that not unexpected behaviour?

I really do like the idea though. As it feels slick that the menu still keeps up with the non-snapping scrolling.

The problem is that it activates heuristically, as in, it detects the direction and activates ahead of the panel actually being in view for more than 50%. Not sure if you want that happening here.

Can you share your thoughts on this?

[akreitals]

Yeah I wasn't sure if it was intentionally omitted or not, obviously feel free to ignore the PR if it does not fit the plugins scope. My personal opinion was that it became confusing when the snapping was disabled but the menu no longer updated as the user scrolled and that it should still represent the state of the panels in view. I felt that this made more sense from a UI perspective.

In terms of how it activates the menu items, this is what I meant by "we might want to consider some kind of additional threshold here". At which point would you consider a panel active? I just let it follow the existing logic, adding an additional threshold, maybe 50% as mentioned, might be more intuitive. What do you think?

Note that I will be travelling until early August but will happily rework this when I return if desired.

[akreitals]

Any thoughts on the above? Any merit in pursuing this further?

[guidobouman]

I think we can leave out the threshold. We only need to disable the direction checks. What you're left with is a check that looks if the panel is visible for more than 50%. Which is exactly what we want.

So you will want the if / else if statements preceding the following line to not work when the plugin is disabled:
https://github.com/guidobouman/jquery-panelsnap/blob/master/jquery.panelSnap.js#L180

The problem: It feels more like only the snapping is disabled, but the rest of the plugin still works, hence the disable method feels a bit awkward. On the other hand, I can't think of a scenario where you want the scrolling to work, snapping disabled and menu disabled as well. The menu makes sense to work at all times.

[akreitals]

Have changed the logic to as suggested above, this feels much smoother, as the plunk demonstrates.

As I said this makes more sense from a UI perspective in my opinion, but I understand the possible confusion, feel free to ignore the PR if you don't feel it is appropriate and I will simply keep the fork for reference.

Edit: forgot to merge, will resubmit.

[guidobouman]

You can just push new commits to the branch, and the pull request will update. Merge in my master branch, and you should be fine.

[guidobouman]

Check the JSLint build errors:

   jquery.panelSnap.js
    176 |      var childNumber;
                              ^ [W004] 'childNumber' is already defined.
    193 |      var $target = self.getPanel(':eq(' + childNumber + ')');
                           ^ [W004] '$target' is already defined.

[guidobouman]

I solved it in a bit more elegant manner, reusing the existing snap checks.