From 97c7b207727fa34ce4b4da8280f6c03cc7ad8905 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Tue, 18 Aug 2026 19:17:04 +0800 Subject: [PATCH] Make model-select's infinite scroll actually work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `` advertised `@infiniteScroll` but nothing behind it ran. The options component rendered ember-infinity's `` against `@infiniteModel`, which was passed as `infiniteModel=this.model` — a `@tracked` property declared on the component and never assigned anywhere. It was therefore always `undefined`, so the loader's `{{#if}}` never opened. ember-infinity was not a dependency either, so the component would have failed to resolve had the branch ever been reached (DEFECTS #105). Paging is native now, with no new dependency: - `loadModels` records the term it loaded for and asks the server for page 1, then decides whether another page exists — preferring the total the server reports and falling back to "the server filled the page", which is all the custom search endpoint gives us. - `loadMoreOptions` is a `dropTask`, so overlapping scrolls collapse into one request. It repeats the current term at the next page and appends the results. - The options component watches ember-basic-dropdown's content element (the thing that actually scrolls) and calls `@onLoadMore` within 32px of the bottom. The listener is torn down through `registerDestructor`. - A spinner row marks the page in flight, styled alongside the existing spinner rules and following the same `ember-model-select__*` naming. A new search restarts at page 1 and keeps the term, so scrolling a filtered list pages through the filtered results rather than the unfiltered ones. Tests: 13 covering a full first page paging into a second, a short page ending paging, the server's reported total ending it, `@infiniteScroll={{false}}`, a new search restarting the sequence, overlapping scrolls dropped, a scroll short of the bottom doing nothing, reaching the bottom of a complete list doing nothing, and the spinner appearing while a page is in flight and going once it lands. The options test file's two loader tests asserted the old inert behaviour and now assert the real thing. Full suite 4975 pass / 0 fail / 0 skip. Coverage 93.73% statements, 89.28% branches, 97.24% functions, 94.13% lines. Two branches in options.js stay partial: the null guards around `scrollable`. Inside a rendered dropdown `closest('.ember-basic-dropdown-content')` always resolves, so they are defensive only — documented in the source rather than suppressed. Two more in model-select.js are pre-existing and recorded as DEFECTS #163: both task permission guards are unreachable, because denying permission also sets `disabled` and power-select refuses to open a disabled trigger. --- addon/components/model-select.hbs | 2 +- addon/components/model-select.js | 97 +++++++++++--- addon/components/model-select/options.hbs | 8 +- addon/components/model-select/options.js | 55 +++++++- .../styles/components/ember-model-select.css | 8 ++ .../components/model-select-test.js | 113 ++++++++++++++++ .../components/model-select/options-test.js | 121 ++++++++++++++++-- 7 files changed, 369 insertions(+), 35 deletions(-) diff --git a/addon/components/model-select.hbs b/addon/components/model-select.hbs index e1eb98c5..46091936 100644 --- a/addon/components/model-select.hbs +++ b/addon/components/model-select.hbs @@ -34,7 +34,7 @@ @onKeydown={{@onKeydown}} @onOpen={{this.onOpen}} @options={{this._options}} - @optionsComponent={{component this.optionsComponent infiniteScroll=this.infiniteScroll infiniteModel=this.model withCreate=this.withCreate}} + @optionsComponent={{component this.optionsComponent infiniteScroll=this.infiniteScroll onLoadMore=this.onLoadMore isLoadingMore=this.loadMoreOptions.isRunning withCreate=this.withCreate}} @placeholder={{@placeholder}} @placeholderComponent={{@placeholderComponent}} @preventScroll={{@preventScroll}} diff --git a/addon/components/model-select.js b/addon/components/model-select.js index ee41ce0c..9c4295dd 100644 --- a/addon/components/model-select.js +++ b/addon/components/model-select.js @@ -128,8 +128,14 @@ export default class ModelSelectComponent extends Component { */ @tracked _options; - @tracked model; @tracked selectedModel; + + /** Paging state for infinite scroll. */ + @tracked page = 1; + @tracked hasMoreOptions = false; + + /** The term the current option list was loaded for, so later pages repeat the same search. */ + lastTerm = null; @tracked permissionRequired = null; @tracked disabled = false; @tracked doesntHavePermissions = false; @@ -201,6 +207,45 @@ export default class ModelSelectComponent extends Component { return; } + this.lastTerm = term; + this.page = 1; + + const query = this.queryFor(term, 1); + const _options = yield this.fetchPage(query); + + this.hasMoreOptions = this.hasMoreAfter(_options, _options.length); + + this._options = createOption + ? // Plain assignment, not `unshiftObjects`: on the customSearchEndpoint path the + // results are a plain array from `results.map(...)` with no Ember array methods. + [createOption, ...this.toPlainArray(_options)] + : _options; + }; + + /** + * Appends the next page to the visible options. Driven by the options component when the + * dropdown is scrolled to the bottom. + */ + @dropTask loadMoreOptions = function* () { + // `hasMoreOptions` is only ever set by a loadModels run that got past its own + // permission/disabled guard, so re-checking those here would be dead code. + if (!this.infiniteScroll || !this.hasMoreOptions) { + return; + } + + const nextPage = this.page + 1; + + // Keep the raw result: `toPlainArray` drops the `meta` the server reports its total in. + const raw = yield this.fetchPage(this.queryFor(this.lastTerm, nextPage)); + const results = this.toPlainArray(raw); + + this.page = nextPage; + this._options = [...this.toPlainArray(this._options), ...results]; + this.hasMoreOptions = results.length > 0 && this.hasMoreAfter(raw, this._options.length); + }; + + /** The query for one page, built the same way whichever page it is. */ + queryFor(term, page) { // query might be an EmptyObject/{{hash}}, make it a normal Object const query = Object.assign({}, this.args.query); @@ -208,13 +253,37 @@ export default class ModelSelectComponent extends Component { set(query, 'query', term); } - let _options; + set(query, this.pageParam, page); + set(query, this.perPageParam, this.pageSize); + return query; + } + + /** Ember arrays, plain arrays and the custom endpoint's `results.map(...)` all arrive here. */ + toPlainArray(options) { + return typeof options.toArray === 'function' ? options.toArray() : [...options]; + } + + /** + * Whether another page is worth asking for. Prefers the total the server reported; falls back + * to "the server filled the page", which is all the custom endpoint gives us. + */ + hasMoreAfter(results, loadedCount) { + const total = get(results, this.totalPagesParam); + + if (typeof total === 'number') { + return loadedCount < total; + } + + return this.toPlainArray(results).length >= this.pageSize; + } + + fetchPage(query) { if (typeof this.args.customSearchEndpoint === 'string') { - const customQuery = (endpoint, query, options = {}) => { + const customQuery = (endpoint, params, options = {}) => { return new Promise((resolve) => { this.fetch - .get(endpoint, query, options) + .get(endpoint, params, options) .then((results) => { let records = results.map((result) => { let modelName = this.args.modelName; @@ -242,22 +311,16 @@ export default class ModelSelectComponent extends Component { }); }; - _options = yield customQuery(this.args.customSearchEndpoint, query); - } else { - set(query, this.pageParam, 1); - set(query, this.perPageParam, this.pageSize); - - _options = yield this.source.query(this.args.modelName, query); + return customQuery(this.args.customSearchEndpoint, query); } - if (createOption) { - // Plain assignment, not `unshiftObjects`: on the customSearchEndpoint path `_options` - // is a plain array from `results.map(...)` and has no Ember array methods. - _options = [createOption, ..._options]; - } + return this.source.query(this.args.modelName, query); + } - this._options = _options; - }; + /** Handed to the options component, which calls it when the list is scrolled to the bottom. */ + @action onLoadMore() { + this.loadMoreOptions.perform(); + } loadDefaultOptions() { const { loadDefaultOptions } = this.args; diff --git a/addon/components/model-select/options.hbs b/addon/components/model-select/options.hbs index a7de9665..e16ab6ce 100644 --- a/addon/components/model-select/options.hbs +++ b/addon/components/model-select/options.hbs @@ -1,4 +1,4 @@ -
    +
    • {{#if this.showLoader}} -
    • - - - +
    • +
      • {{/if}}
    \ No newline at end of file diff --git a/addon/components/model-select/options.js b/addon/components/model-select/options.js index edb3c9fc..6c17f471 100644 --- a/addon/components/model-select/options.js +++ b/addon/components/model-select/options.js @@ -1,9 +1,58 @@ import Component from '@glimmer/component'; -import { computed } from '@ember/object'; +import { action } from '@ember/object'; +import { registerDestructor } from '@ember/destroyable'; +/** How close to the bottom of the list counts as "scrolled to the end", in pixels. */ +const LOAD_MORE_THRESHOLD = 32; + +/** + * The dropdown's option list, with infinite scroll. + * + * This used to render ember-infinity's `` against an `@infiniteModel` that was + * never assigned — and ember-infinity was not even a dependency, so the component would not have + * resolved had the loader ever been reached. Paging is native now: when the dropdown is scrolled + * to the bottom and there is another page to fetch, `@onLoadMore` is called. + */ export default class ModelSelectOptionsComponent extends Component { - @computed('args.{infiniteScroll,infiniteModel,select.loading}') + scrollable = null; + + constructor() { + super(...arguments); + registerDestructor(this, () => this.#stopWatching()); + } + + /** The spinner shows while a further page is on its way. */ get showLoader() { - return this.args.infiniteScroll && this.args.infiniteModel && !this.args.select.loading; + return Boolean(this.args.infiniteScroll && this.args.isLoadingMore); + } + + @action watchForScrollEnd(element) { + // ember-basic-dropdown scrolls its content element, not this list. power-select only ever + // renders this component inside that element, so in practice the lookup always resolves — + // the null guards below are defensive and are not reachable from a rendered dropdown. + this.scrollable = element.closest('.ember-basic-dropdown-content'); + + if (this.scrollable) { + this.scrollable.addEventListener('scroll', this.onScroll, { passive: true }); + } + } + + @action onScroll() { + if (!this.args.infiniteScroll || typeof this.args.onLoadMore !== 'function') { + return; + } + + const { scrollTop, clientHeight, scrollHeight } = this.scrollable; + + if (scrollHeight - scrollTop - clientHeight <= LOAD_MORE_THRESHOLD) { + this.args.onLoadMore(); + } + } + + #stopWatching() { + if (this.scrollable) { + this.scrollable.removeEventListener('scroll', this.onScroll); + this.scrollable = null; + } } } diff --git a/addon/styles/components/ember-model-select.css b/addon/styles/components/ember-model-select.css index 29fe7426..48775e56 100644 --- a/addon/styles/components/ember-model-select.css +++ b/addon/styles/components/ember-model-select.css @@ -25,6 +25,14 @@ animation: ember-model-select-spin 1s infinite linear; } +/* The row that carries the spinner while a further page of options is being fetched. */ +.ember-model-select__loading-more { + display: flex; + align-items: center; + justify-content: center; + padding: 8px 0; +} + .ember-model-select__spinner > circle { stroke-opacity: 0.1; } diff --git a/tests/integration/components/model-select-test.js b/tests/integration/components/model-select-test.js index 530abb8f..cb43c907 100644 --- a/tests/integration/components/model-select-test.js +++ b/tests/integration/components/model-select-test.js @@ -5,6 +5,7 @@ import { hbs } from 'ember-cli-htmlbars'; import Service from '@ember/service'; import { A } from '@ember/array'; import { selectChoose, selectSearch, getDropdownItems } from 'ember-power-select/test-support'; +import { clickTrigger } from 'ember-power-select/test-support/helpers'; const DRIVERS = [ { id: 'drv_1', name: 'Alex Driver' }, @@ -423,4 +424,116 @@ module('Integration | Component | model-select', function (hooks) { assert.ok(find('.fleetbase-model-select'), 'no handler is required'); }); }); + // Infinite scroll never worked: `@infiniteModel` was handed a `model` field that was declared + // and never assigned, and the `` it fed came from ember-infinity, which was + // not a dependency. Paging is native now. + module('infinite scroll', function () { + function page(n, size) { + return A(Array.from({ length: size }, (_, i) => ({ id: `drv_${n}_${i}`, name: `Driver ${n}-${i}` }))); + } + + // Dispatch only. Assigning scrollTop fires its own native scroll event, so doing both + // would trigger the loader twice per call. + async function scrollToBottom() { + find('.ember-basic-dropdown-content').dispatchEvent(new Event('scroll')); + await settled(); + } + + test('a full first page is followed by a second when the list is scrolled to the end', async function (assert) { + respondWith = (modelName, query) => page(query.page, 25); + + await render(hbs``); + await clickTrigger(); + + assert.strictEqual(queries.length, 1, 'the first page is requested on open'); + assert.strictEqual(queries[0].query.page, 1); + assert.strictEqual(queries[0].query.limit, 25, 'with the configured page size'); + + await scrollToBottom(); + + assert.strictEqual(queries.length, 2, 'reaching the bottom asks for the next page'); + assert.strictEqual(queries[1].query.page, 2, 'and it is the page after'); + assert.strictEqual(findAll('.ember-power-select-option').length, 50, 'the results are appended, not replaced'); + }); + + test('a short page means there is nothing more to ask for', async function (assert) { + respondWith = () => page(1, 3); + + await render(hbs``); + await clickTrigger(); + await scrollToBottom(); + + assert.strictEqual(queries.length, 1, 'a page that did not fill is the last one'); + }); + + test('the reported total decides when to stop', async function (assert) { + respondWith = (modelName, query) => { + const records = page(query.page, 25); + records.meta = { total: 30 }; + return records; + }; + + await render(hbs``); + await clickTrigger(); + await scrollToBottom(); + + assert.strictEqual(queries.length, 2, '25 of 30 loaded, so one more page is fetched'); + + await scrollToBottom(); + + assert.strictEqual(queries.length, 2, 'and once past the total, no further requests'); + }); + + test('@infiniteScroll={{false}} never asks for another page', async function (assert) { + respondWith = (modelName, query) => page(query.page, 25); + + await render(hbs``); + await clickTrigger(); + await scrollToBottom(); + + assert.strictEqual(queries.length, 1, 'paging is off'); + }); + + test('a new search starts again from page one', async function (assert) { + respondWith = (modelName, query) => page(query.page, 25); + + await render(hbs``); + await clickTrigger(); + await scrollToBottom(); + assert.strictEqual(queries[1].query.page, 2); + + await selectSearch(TRIGGER, 'hauler'); + + const latest = queries[queries.length - 1]; + assert.strictEqual(latest.query.page, 1, 'the new term is requested from the first page'); + assert.strictEqual(latest.query.query, 'hauler', 'carrying the search term'); + + await scrollToBottom(); + + const afterScroll = queries[queries.length - 1]; + assert.strictEqual(afterScroll.query.page, 2, 'and the next page keeps that term'); + assert.strictEqual(afterScroll.query.query, 'hauler'); + }); + + test('scrolling while a page is already in flight does not stack requests', async function (assert) { + let release; + respondWith = (modelName, query) => { + if (query.page === 1) return page(1, 25); + return new Promise((resolve) => (release = () => resolve(page(2, 25)))); + }; + + await render(hbs``); + await clickTrigger(); + + const content = find('.ember-basic-dropdown-content'); + content.dispatchEvent(new Event('scroll')); + content.dispatchEvent(new Event('scroll')); + content.dispatchEvent(new Event('scroll')); + + assert.strictEqual(queries.length, 2, 'the task drops the overlapping scrolls'); + + release(); + await settled(); + }); + }); }); diff --git a/tests/integration/components/model-select/options-test.js b/tests/integration/components/model-select/options-test.js index 928d78cb..26bd9eb9 100644 --- a/tests/integration/components/model-select/options-test.js +++ b/tests/integration/components/model-select/options-test.js @@ -1,6 +1,6 @@ import { module, test } from 'qunit'; import { setupRenderingTest } from 'dummy/tests/helpers'; -import { render, click, findAll, find } from '@ember/test-helpers'; +import { render, click, findAll, find, settled, waitFor } from '@ember/test-helpers'; import { hbs } from 'ember-cli-htmlbars'; import Service from '@ember/service'; import { A } from '@ember/array'; @@ -63,26 +63,25 @@ module('Integration | Component | model-select/options', function (hooks) { ); }); - // DEFECT (see DEFECTS.md #105): showLoader is `infiniteScroll && infiniteModel && !select.loading`, - // and passes `infiniteModel=this.model` — a @tracked property that is declared but - // NEVER assigned anywhere in model-select.js. infiniteModel is therefore always undefined, so the - // InfinityLoader branch of this component is unreachable and ModelSelect's infinite scroll is inert. - test('the infinity loader never renders, even with infinite scroll on', async function (assert) { + // #105 used to live here: this component rendered ember-infinity's against an + // @infiniteModel that was never assigned (and ember-infinity was not a dependency), so the loader + // was unreachable and paging was inert. Paging is native now, so these assert the real thing. + test('nothing is loading when the list first opens', async function (assert) { await render(hbs``); await click(TRIGGER); const items = findAll('ul[role="listbox"] > li'); assert.strictEqual(items.length, 1, 'only the options list item is rendered'); - assert.notOk(find('ul[role="listbox"] .fleetbase-loader'), 'no loader is rendered despite infiniteScroll defaulting to true'); + assert.dom('.ember-model-select__loading-more').doesNotExist('no spinner while the list sits idle'); }); - test('no infinity loader is rendered when infinite scroll is off', async function (assert) { + test('no loading row is rendered when infinite scroll is off', async function (assert) { await render(hbs``); await click(TRIGGER); const items = findAll('ul[role="listbox"] > li'); assert.strictEqual(items.length, 1, 'only the options list item is rendered'); - assert.notOk(find('ul[role="listbox"] .fleetbase-loader'), 'no loader is rendered'); + assert.dom('.ember-model-select__loading-more').doesNotExist(); }); test('an empty result still renders the listbox', async function (assert) { @@ -108,4 +107,108 @@ module('Integration | Component | model-select/options', function (hooks) { assert.strictEqual(changes[0].id, 'drv_2'); assert.notOk(find('ul[role="listbox"]'), 'the list closed itself'); }); + // The list is scrolled by ember-basic-dropdown's content element, which is what the component + // attaches its listener to. These drive that element directly. + module('scrolling the dropdown', function (hooks) { + let pages; + + // A short page tells there is nothing more to fetch, which would make every + // assertion below unfailable. These serve a full page so paging stays live. + const FULL_PAGE = Array.from({ length: 25 }, (unused, index) => ({ id: `drv_${index}`, name: `Driver ${index}` })); + + hooks.beforeEach(function () { + pages = []; + respondWith = (modelName, query) => { + pages.push(query.page); + return A(FULL_PAGE.slice()); + }; + }); + + function scrollContent({ top, clientHeight, scrollHeight }) { + const content = find('.ember-basic-dropdown-content'); + Object.defineProperty(content, 'scrollTop', { value: top, configurable: true }); + Object.defineProperty(content, 'clientHeight', { value: clientHeight, configurable: true }); + Object.defineProperty(content, 'scrollHeight', { value: scrollHeight, configurable: true }); + content.dispatchEvent(new Event('scroll')); + return settled(); + } + + test('a scroll short of the bottom asks for nothing', async function (assert) { + await render(hbs``); + await click(TRIGGER); + const afterOpen = pages.length; + + await scrollContent({ top: 0, clientHeight: 100, scrollHeight: 1000 }); + + assert.strictEqual(pages.length, afterOpen, 'still 900px from the end, so no further page is fetched'); + assert.strictEqual(pages.at(-1), 1, 'only the first page has been asked for'); + }); + + test('a scroll within the threshold of the bottom fetches the next page', async function (assert) { + await render(hbs``); + await click(TRIGGER); + const afterOpen = pages.length; + + await scrollContent({ top: 880, clientHeight: 100, scrollHeight: 1000 }); + + assert.strictEqual(pages.length, afterOpen + 1, '20px from the end is close enough'); + assert.strictEqual(pages.at(-1), 2, 'and it is the second page that is asked for'); + }); + + test('reaching the bottom of a complete list asks for nothing', async function (assert) { + // A short first page means the server has already sent everything, so hitting the end + // of it must not start a second request. + respondWith = (modelName, query) => { + pages.push(query.page); + return A(DRIVERS.slice()); + }; + + await render(hbs``); + await click(TRIGGER); + const afterOpen = pages.length; + + await scrollContent({ top: 900, clientHeight: 100, scrollHeight: 1000 }); + + assert.strictEqual(pages.length, afterOpen, 'the list is already complete'); + }); + + test('the spinner shows while the next page is in flight, then goes away', async function (assert) { + // Hold the second page open so the loading state can be observed rather than raced past. + let releaseSecondPage; + respondWith = (modelName, query) => { + pages.push(query.page); + + if (query.page === 1) { + return A(FULL_PAGE.slice()); + } + + return new Promise((resolve) => { + releaseSecondPage = () => resolve(A(DRIVERS.slice())); + }); + }; + + await render(hbs``); + await click(TRIGGER); + assert.dom('.ember-model-select__loading-more').doesNotExist('nothing in flight yet'); + + const scrolled = scrollContent({ top: 880, clientHeight: 100, scrollHeight: 1000 }); + await waitFor('.ember-model-select__loading-more'); + assert.dom('.ember-model-select__loading-more .ember-model-select__spinner').exists('the spinner marks the page being fetched'); + + releaseSecondPage(); + await scrolled; + + assert.dom('.ember-model-select__loading-more').doesNotExist('and it goes once the page has landed'); + }); + + test('with infinite scroll off, reaching the bottom fetches nothing', async function (assert) { + await render(hbs``); + await click(TRIGGER); + const afterOpen = pages.length; + + await scrollContent({ top: 900, clientHeight: 100, scrollHeight: 1000 }); + + assert.strictEqual(pages.length, afterOpen, 'paging is off entirely'); + }); + }); });