-
Notifications
You must be signed in to change notification settings - Fork 86
Description
Description
<vaadin-grid>
has this great function that allows the Grid to scroll to a given index. It can be used to make sure that a (programmatically) selected row is visible in the Grid. It can be used like this:
class MyView extends LitElement {
render() {
return html `
<vaadin-grid id='list' .items=${this.items} @active-item-changed=${this.activeItem}>
...
`;
}
activeItem(event) {
const item = event.detail.value;
this.shadowRoot.getElementById('list').selectedItems = item ? [this.item] : [];
if (item) {
this.shadowRoot.getElementById('list')._scrollToIndex(this.items.indexOf(item));
}
}
}
customElements.define('my-view', ContentView);
The feature does pretty much what it says. The problem for me is that _scrollToIndex will always scroll so that the index given is the topmost row in the visible range. This causes quite some unexpected jumpyness in many cases. Consider that the row that you clicked on is already visible - the Grid will still scroll to force that row to be the first one in view. Consider that the row selected is under what is visible - grid will not scroll only a bit down so that it gets into view, but the maximum amount to put it at the top. When using the UI, I often get a "what happened?" -feeling when it jumps unexpectedly.
The best course of action would be to let the developer choose how it would scroll. This could be done by passing a second parameter to the function. I see as there are four different options: index to top (like it is today), index to bottom, index to middle (nice to have a row of focus dead center) and shortest scroll possible.
I think the best default value would be the last, scroll the least amount possible. If the row is already fully visible, then don't scroll at all. If the row is below the current viewport, scroll down the minimum amount so that the row is the last one visible in the viewport. If the row is above, scroll up so that it is the first row visible. I think this would be much more intuitive for end users.
To summarize on my suggestion.
- make scrollToIndex(index) use shortest scroll method
- give a second parameter, scrollToIndex(index, strategy) with shortest, top, bottom and middle as options to let the developer decide on the behaviour.
I know _scrollToIndex(index) is a protected function. It is however very handy. Vaadin Framework 8 and earlier had that same feature, and there is already vaadin/vaadin-grid#1169 suggesting to exposing it. This is feature is awesome in many cases.
Expected outcome
The Grid scrolls the least amount to make the index visible, or the scroll target is definable (top, middle, bottom, shortest)
Actual outcome
The Grid always scrolls so that the grid is at top, no matter if the row is already visible or below the current visible rows.
Live Demo
Steps to reproduce
See code above or referred github
Browsers Affected
- all