asvd / dragscroll

micro library for drag-n-drop scrolling style
http://asvd.github.io/dragscroll/
MIT License
1.1k stars 166 forks source link

Make text selectable if no scrollbar after .reset() #12

Closed NeXTs closed 8 years ago

NeXTs commented 8 years ago

Consider such case:

User opens page at browser window which is 50% of screen width. Page contains full-width table (100% page width) but it's content not fit horizontally so horizontal scrollbar appears and dragscroll plugin inits. User reveals browser window to fullscreen, table successfully fits page and hasn't horizontall scrollbar anymore. Since table fits now there are no sense to use dragscroll so I do dragscroll.reset() (after resize ends) hoping that all event listeners will be removed and user will be able to select text. But it doesn't. Seems that events are not removed so user not able to select text.

asvd commented 8 years ago

Makes sence. But does the table contains the data which should be selectable? If yes, it should not be scrolled by dragging. This is what you need to decide in the beginning and make a choice on what dragging does - scrolls the content or selects the text. Otherwise, making a user to expand the table in order to enable the selection is not that predictable (user might simply have not big enough screen for that).

NeXTs commented 8 years ago

I want to combine them and take the best from both sides. If scrollbar exists I'll make cursor: grab and allow to drag table's content (of course text wouldn't be selectable), otherwise I'll make default cursor and allow user to select text.

This should be pretty convenient and predictable as for me.

asvd commented 8 years ago

Pointer shape would give user a hint about if a content can be either selected or dragged, but there is no way for a user to figure out that expanding the window would make the content selectable. Therefore changing the behaviour depending on window size makes no sence: the viewport size has no relation to user's need of selecting the content, and these two UX issues should be solved independently. Following this logic, I don't find your solution good. Similarly in dragscroll the behaviour does not fall back to content selection in case if viewport cannot be scrolled: an area set up for scrolling by dragging will always remain like this.

Here you have the two options:

  1. You can check if the viewport is scrollable upon window resize, and if not, remove the dragscroll class and invoke dragscroll.reset(). This will restore the content selection. As far as I understand, this is what you would suggest with this ticket as a feature for dragscroll, which hovewer will not be implemented, for the reasons given above.
  2. You can add the nochilddrag attriubte to the scrollable element, and it will only be dragable if you drag the element itself, but not it's subchildren. Therefore a table inside that area will be selectable as usual and dragging will be disabled there, but user will still be able to drag the viewport by pulling the space next to the table. I am currently working on a demo with nearly the same functionality: http://asvd.github.io/jailed/demos/web/process/
NeXTs commented 8 years ago

Take a look http://codepen.io/NeXTs/pen/bEZQKX

First option is what I want but .reset() doesn't restore text selection. BTW I like second option too.

asvd commented 8 years ago

Very strange, looks like a bug in dragscroll. Will take a look at it, thanks!

asvd commented 8 years ago

Confirmed, that was due to my misunderstanding of how HTMLCollection works, will fix it soon

asvd commented 8 years ago

The problem is the subject of #4

@NeXTs This ticket remains open for the case if you still unsure about the UX issues we discussed above

NeXTs commented 8 years ago

this may fix the issue

dragged = Array.prototype.slice.call(_document.getElementsByClassName('dragscroll'));
asvd commented 8 years ago

nice solution, thanks :+1:

(but due to Array.prototype.slice.call farewell "734 bytes minified" :disappointed: )

NeXTs commented 8 years ago

:smile:

dragged = [].slice.call(_document.getElementsByClassName('dragscroll'));
NeXTs commented 8 years ago

The only caveat: this works for IE9+ only If you need support of IE8 and lower then workaround is more verbose, unfortunately

dragged.length = 0;
var _dragged = _document.getElementsByClassName('dragscroll');
for (var i = 0; ii = _dragged.length; i < ii; i++) dragged.push(_dragged[i]);
asvd commented 8 years ago

To be honest, never tested it on older ie's, but if other pieces of code work there - then it might make sence to proceed with the latter solution. I would only omit caching the length into ii (assuming the number of scrollable elements on a page is not that big).

NeXTs commented 8 years ago

Sure, it's up to you

NeXTs commented 8 years ago

Hey Dmitry Please update github release to v0.0.6 so that CDN's pick up last update

asvd commented 8 years ago

ok, will do this evening

asvd commented 8 years ago

done https://github.com/asvd/dragscroll/releases/tag/v0.0.6