Focused state of dom node is not persisted when scrolling

[lukesargeant]

When I scroll a list of rows containing input fields, the focused input field does not move with its row. Instead the dom node that is being re-used (and had its model change to reflect another row) holds on to the focus state.

As a consumer of this library it would be nice not to be concerned with this problem such that the virtualization is hidden from me. Would this be a reasonable case to support? I understand there could be a number of complexities around changing focus and potentially suppressing events, or should the library accept this limitation?

[jasonmit]

Any chance you can throw together an example?

[lukesargeant]

I don't think ember-twiddle supports addons so it's not so simple to knock up a working example, but it's simple to build an example yourself. This just needs the each replaced with virtual-each. Then try to focus an input, and start scrolling. https://ember-twiddle.com/e532ba03feb983ad49d5

[jasonmit]

Unsure how this can be solved given it would be fighting glimmer, which only updates fragments of the DOM that changed. In this case, the input doesn't change and there for doesn't rerender so the focus is never lost.

That said, I'll implement an example on how you can achieved, it just won't be baked in to the lib. I'll post it tomorrow when I have a chance to write it out.

[lukesargeant]

Yes, unfortunately it would need to detect focus, and move the focus whilst suppressing the focus/blur etc events. Pretty tricky :-S
Another option would be to treat rows with focus differently to normal rows and de-optimise performance by actually moving it in the dom. Tough call.

[jasonmit]

@lukesargeant unsure how that would look, feel free to spike a PR it if you have an idea/time. Otherwise, will keep the issue opened to let it marinate.

[bitsoflogic]

I'm just now hitting this as well. While this certainly isn't anything as sophisticated as what was being proposed, how about simply exposing when the startAt and endAt properties change? By providing something like onIndexChange, we could implement a work-around until a better solution appears. Thoughts?

[bitsoflogic]

In case others find this useful for now, here's my work-around:

component:

actions: {
  virtualIndexChanged: function() {
    document.activeElement.blur();
  }

template:

 {{virtual-each 
   onIndexChange=(action 'virtualIndexChanged')
   ... 

virtual-each.js:

 visibleItems: computed('_startAt', '_visibleItemCount', '_items', {
    get() {
      const items = get(this, '_items');
      const startAt = get(this, '_startAt');
      const _visibleItemCount = get(this, '_visibleItemCount');
      const itemsLength = get(items, 'length');
      const endAt = Math.min(itemsLength, startAt + _visibleItemCount);
      const prevStartAt = get(this, '_prevStartAt');
      const prevEndAt = get(this, '_prevEndAt');
      const onScrollBottomed = this.attrs.onScrollBottomed;
      const onIndexChange = this.attrs.onIndexChange;

      if (typeof onScrollBottomed === 'function' && (startAt + _visibleItemCount - EXTRA_ROW_PADDING) >= itemsLength) {
        onScrollBottomed(startAt, endAt);
      }

      if (typeof onIndexChange === 'function' && (startAt !== prevStartAt || endAt !== prevEndAt)) {
        onIndexChange(startAt, endAt);
      }

      set(this, '_prevStartAt', startAt);
      set(this, '_prevEndAt', endAt);

      return items.slice(startAt, endAt).map((item, index) => {
        return {
          raw: item,
          actualIndex: startAt + index,
          virtualIndex: index
        };
      });
    }
  }).readOnly(),

  ... 

 init() {
    this._super(...arguments);

    this.setProperties({
      _items: emberArray(),
      _startAt: 0,
      _totalHeight: 0,
      _scrolledByWheel: false,
      _prevStartAt: 0,
      _prevEndAt: 0
    });
  },

[jasonmit]

@bitsoflogic feel free to send a PR to add support for optionally invoking an onScrolled action

[lukesargeant]

@bitsoflogic +1 for a simple solution to a tough problem.

[dephora]

So I'm running into a similar issue. I've adopted the code above (updated so it's in line with the current version of the addon), however it doesn't seem to be working. I'm on 2.13 and I'm not sure if it's failing because Glimmer has been changed since the fix was posted.

Below is a gif of the issue and the edited code. Everything appears to be firing when it should.

visibleItems: computed('_startAt', '_itemCount', '_items.[]', 'bufferSize', {
    get() {
      let { _items, _startAt, _itemCount } = getProperties(this, '_items', '_startAt', '_itemCount');
      let bufferSize = get(this, 'bufferSize');
      let itemsLength = get(_items, 'length');
      let endAt = Math.min(itemsLength, _startAt + _itemCount);
      let { onScrollBottomed } = this.attrs;

      // new code
      let prevEndAt = get(this, '_prevEndAt');
      let prevStartAt = get(this, '_prevStartAt');
      let { onIndexChange } = this.attrs;
      // end new code


      if (typeof onScrollBottomed === 'function' && (_startAt + _itemCount - bufferSize) >= itemsLength) {
        this._scrollBottomedTimeout = run.later(() => onScrollBottomed(_startAt, endAt), 5);
      }

      // new code
      if (typeof onIndexChange === 'function' && (_startAt !== prevStartAt || endAt !== prevEndAt)) {
        console.log('onIndexChange')
        onIndexChange(_startAt, endAt);
      }

      set(this, '_prevStartAt', _startAt);
      set(this, '_prevEndAt', endAt);
      console.log(`Start: ${get(this, '_prevStartAt')}`)
      console.log(`End: ${get(this, '_prevEndAt')}`)
      // end new code

      return _items.slice(_startAt, endAt).map((item, index) => {
        return {
          raw: item,
          actualIndex: _startAt + index,
          virtualIndex: index
        };
      });
    }
  }).readOnly(),

...

init() {
    this._super(...arguments);

    setProperties(this, {
      _items: emberArray(),
      _startAt: 0,
      _totalHeight: 0,
      _scrolledByWheel: false,
      _prevStartAt: 0,
      _prevEndAt: 0
    });
  },

Any recommendations would be appreciated, I can link to the repo if necessary.