search

mzubala / jquery-custom-scrollbar

189 stars 102 forks source link

Fixes #10, #9 #11

Closed ekzobrain closed 11 years ago

ekzobrain commented 11 years ago

I implemented fixes for mobile zoom in/out and keyboard control problems.

For keyboard controls I implemented two behaviors: the most browser-like (currently active) and hover(), behaving exactly as you plugin previously did, but with honoring form elements (currently commented out). I think that browser-like approach (element is scrollable with keyboad when it is active/focused) is the best to use, till it provides really native behavior and requires much less code.

mzubala commented 11 years ago

After your changes keyboard scrolling when scrollbars are nested does not work as expected. When I click on nested component and scroll using keyboard also parent container is scrolled.

Just in case you haven't seen it - demos/nested.html is a test for nested scrollbars. You can observe the wrong behavior in this example.

mzubala commented 11 years ago

Your zoom in/out change works well. I pulled it into master. Thanks!

mzubala commented 11 years ago

I modified your code a bit to support nested scrollbars. Also when focus was in a text field and you would press up arrow scrolling occurred. I fixed both problems in my last commit to this branch. Pls let me know your thoughts.

ekzobrain commented 11 years ago

Yes, seem like keydown event bubbles up the DOM tree and fires on focusable elements. That's not what is said in jQuery docs: "The keydown event is sent to an element when the user first presses a key on the keyboard. It can be attached to any element, but the event is only sent to the element that has the focus" (http://api.jquery.com/keydown/). "Has the focus" and "focusable" are not the same, because only one element can have focus in a moment. So, we should believe to browsers, but not docs :) Your fix is generally correct, but where is a bit better approach: just place event-handler operations into this condition: "if (document.activeElement === _this.$element[0])", "return false" should be removed. It ensures that event is only handled on currently active scrollable element (this excludes form elements and other focusable elements that may cause incorrect behavior (links or user-defined, for example)) and doesn't break event bubbling if user wants to use it somewhere else.