dom-helpers icon indicating copy to clipboard operation
dom-helpers copied to clipboard

util/scrollTo.js: add option to scroll selected item to the top of sc…

Open maps82 opened this issue 9 years ago • 8 comments

I added a parameter to the scrollTo function to scroll selected to the top of scrollParent, instead to the bottom. Would be great if that could be merged :)

maps82 avatar Aug 04 '16 13:08 maps82

I'm all for adding this, does this handle scrolling up from below the selected item with an inverse logic to the default? meaning will the selected item is below the viewport it scrolls just enough to pull it into view (ie the bottom) when the item is a one the viewport it scrolls to top. I'd imagine this would want to do the opposite in both cases?

jquense avatar Aug 04 '16 14:08 jquense

I'm not sure. I currently use it with a slightly modified version of react-widgets, to get the timepicker scroll automatically to a given time when opening it (as long is no selection is made)

Do you have a simple test scenario for me?

maps82 avatar Aug 04 '16 15:08 maps82

I'd just look at the behavior in RW: http://jquense.github.io/react-widgets/docs/#/datetime-picker?_k=ymbdaq use the arrow keys to scroll up and down in the time list and you'll see what I mean.

jquense avatar Aug 04 '16 17:08 jquense

Hi. I'm still not exactly sure what you mean, but I can keyboard navigate in my modified RW version exactly as in yours. It's just that I pass a new prop through DateTimePicker to TimeList, that preselects 17h (in this example). It's not a real preselection, as the value is still empty, it's just that 17h is presented as first option (although you can scroll up to earlier times). To achieve that, this patch is needed.

If I press arrow up/down, the focusItem will NOT stick at the top, the behavior is identical to your example.

Also I tested that the patched scrollTo function always pulls the selected item into the viewport (no matter if it's below or above)

ezgif-3860083262

maps82 avatar Aug 04 '16 18:08 maps82

I think we are miss understanding each other. The default behavior of scrollTo is to only pull the item into view to it's closest edge. your patch only pulls the Item to the top of the viewport. that is weird behavior when scroll ing up from below the item.

jquense avatar Aug 04 '16 18:08 jquense

Hm, but that's exactly what I need in my case. It always puts the wanted item at the top, regardless if it's below or above the viewport. As far as possible of course - if it's one of the last items of the list it's not possible. Is there anything I can do to make it more generic to get the pr accepted?

Do you see the use case for RW? (see gif above: the user selects a timespan - usually business hours)

maps82 avatar Aug 04 '16 19:08 maps82

you are only thinking of the one case for RW, which is when you open the popup from closed. the same logic doesn't make sense for keyboard navigation...I'm actually curious how you do it in the one case but not the other. both scrolling hen open and scrolling when pressing arrowKey up/down.

jquense avatar Aug 04 '16 19:08 jquense

I'm actually curious how you do it in the one case but not the other. both scrolling when open and scrolling when pressing arrowKey up/down.

You can see this here

maps82 avatar Aug 04 '16 20:08 maps82