From be4d77e765d8ec11ef6364156962592ff8151004 Mon Sep 17 00:00:00 2001 From: Matthew Beale Date: Mon, 11 Sep 2023 13:19:47 -0400 Subject: [PATCH] Rewrite VerticalCollection to a GlimmerComponent --- .../data-view/elements/occluded-content.js | 6 +- .../data-view/elements/virtual-component.js | 8 +- .../-private/data-view/radar/dynamic-radar.js | 16 +- addon/-private/data-view/radar/radar.js | 2 +- .../vertical-collection/component.js | 174 +++++++++++------- .../vertical-collection/template.hbs | 1 + addon/helpers/vertical-collection/call.js | 5 + app/helpers/vertical-collection/call.js | 1 + package.json | 1 + .../record-array/template.hbs | 2 +- yarn.lock | 2 +- 11 files changed, 136 insertions(+), 82 deletions(-) create mode 100644 addon/helpers/vertical-collection/call.js create mode 100644 app/helpers/vertical-collection/call.js diff --git a/addon/-private/data-view/elements/occluded-content.js b/addon/-private/data-view/elements/occluded-content.js index c77c1062..6b000406 100644 --- a/addon/-private/data-view/elements/occluded-content.js +++ b/addon/-private/data-view/elements/occluded-content.js @@ -1,11 +1,13 @@ -import { set } from '@ember/object'; import { DEBUG } from '@glimmer/env'; +import { tracked } from '@glimmer/tracking'; import document from '../../utils/document-shim'; let OC_IDENTITY = 0; export default class OccludedContent { + @tracked element; + constructor(tagName) { this.id = `OC-${OC_IDENTITY++}`; this.isOccludedContent = true; @@ -72,6 +74,6 @@ export default class OccludedContent { } destroy() { - set(this, 'element', null); + this.element = null; } } diff --git a/addon/-private/data-view/elements/virtual-component.js b/addon/-private/data-view/elements/virtual-component.js index 878ba3d9..4578a1e8 100644 --- a/addon/-private/data-view/elements/virtual-component.js +++ b/addon/-private/data-view/elements/virtual-component.js @@ -1,4 +1,5 @@ import { set } from '@ember/object'; +import { tracked } from '@glimmer/tracking'; import { assert } from '@ember/debug'; import { DEBUG } from '@glimmer/env'; @@ -7,6 +8,9 @@ import document from '../../utils/document-shim'; let VC_IDENTITY = 0; export default class VirtualComponent { + @tracked upperBound; + @tracked lowerBound; + constructor(content = null, index = null) { this.id = `VC-${VC_IDENTITY++}`; @@ -79,8 +83,8 @@ export default class VirtualComponent { } destroy() { - set(this, 'upperBound', null); - set(this, 'lowerBound', null); + this.upperBound = null; + this.lowerBound = null; set(this, 'content', null); set(this, 'index', null); } diff --git a/addon/-private/data-view/radar/dynamic-radar.js b/addon/-private/data-view/radar/dynamic-radar.js index 18d3b66e..35344fb4 100644 --- a/addon/-private/data-view/radar/dynamic-radar.js +++ b/addon/-private/data-view/radar/dynamic-radar.js @@ -1,4 +1,5 @@ import { DEBUG } from '@glimmer/env'; +import { tracked } from '@glimmer/tracking'; import Radar from './radar'; import SkipList from '../skip-list'; @@ -6,16 +7,17 @@ import roundTo from '../utils/round-to'; import getScaledClientRect from '../../utils/element/get-scaled-client-rect'; export default class DynamicRadar extends Radar { - constructor(parentToken, options) { - super(parentToken, options); + @tracked _firstItemIndex=0; + @tracked _lastItemIndex=0; + + @tracked _totalBefore=0; + @tracked _totalAfter=0; - this._firstItemIndex = 0; - this._lastItemIndex = 0; + @tracked _minHeight = Infinity; - this._totalBefore = 0; - this._totalAfter = 0; + constructor(parentToken, options) { + super(parentToken, options); - this._minHeight = Infinity; this._nextIncrementalRender = null; diff --git a/addon/-private/data-view/radar/radar.js b/addon/-private/data-view/radar/radar.js index 5b1c919e..f7371a44 100644 --- a/addon/-private/data-view/radar/radar.js +++ b/addon/-private/data-view/radar/radar.js @@ -687,7 +687,7 @@ export default class Radar { this._prevFirstItemIndex += numPrepended; this._prevLastItemIndex += numPrepended; - this.orderedComponents.forEach((c) => set(c, 'index', get(c, 'index') + numPrepended)); + this.orderedComponents.forEach((c) => set(c, 'index', c.index + numPrepended)); this._firstReached = false; diff --git a/addon/components/vertical-collection/component.js b/addon/components/vertical-collection/component.js index 91fa1e9d..15c31f39 100644 --- a/addon/components/vertical-collection/component.js +++ b/addon/components/vertical-collection/component.js @@ -2,20 +2,17 @@ import { DEBUG } from '@glimmer/env'; import { assert } from '@ember/debug'; /* - * This lint rule is incorrectly trigged by the presence of native - * classes in this file. Computed properties are not actually used - * as part of the native classes. - * - * This disable can likely be removed once the base component is - * refactored to a native class (and likely Glimmer component). + * Computed properties are used in this file to maintain backwards + * compatibility with array proxies. */ -/* eslint-disable ember/no-computed-properties-in-native-classes, ember/no-component-lifecycle-hooks */ -import { empty, readOnly } from '@ember/object/computed'; +/* eslint-disable ember/no-computed-properties-in-native-classes */ +import { action, computed, get } from '@ember/object'; +import { dependentKeyCompat } from '@ember/object/compat'; -import Component from '@ember/component'; -import { get, computed } from '@ember/object'; +import Component from '@glimmer/component'; import { run } from '@ember/runloop'; import layout from './template'; +import { setComponentTemplate } from '@ember/component'; import { ViewportContainer } from '../../-private'; import { scheduler, Token } from 'ember-raf-scheduler'; @@ -187,20 +184,19 @@ class Visualization { * END DEBUG HELPERS */ -const VerticalCollection = Component.extend({ - layout, - - tagName: '', +class VerticalCollection extends Component { /** * Property name used for storing references to each item in items. Accessing this attribute for each item * should yield a unique result for every item in the list. * - * @property key + * @property key // TODO also an argument, how to represent this? * @type String * @default '@identity' */ - key: '@identity', + get key() { + return this.args.identity ?? '@identity'; + } // –––––––––––––– Required Settings @@ -208,21 +204,19 @@ const VerticalCollection = Component.extend({ * Estimated height of an item to be rendered. Use best guess as this will be used to determine how many items * are displayed virtually, before and after the vertical-collection viewport. * - * @property estimateHeight + * @argument estimateHeight // TODO: How to represent this * @type Number * @required */ - estimateHeight: null, /** * List of objects to svelte-render. * Can be called like ``. * - * @property items + * @argument items // TODO: How to represent this * @type Array * @required */ - items: null, // –––––––––––––– Optional Settings /** @@ -230,10 +224,12 @@ const VerticalCollection = Component.extend({ * If true, the vertical-collection will assume that items' heights are always equal to estimateHeight; * this is more performant, but less flexible. * - * @property staticHeight + * @property staticHeight // TODO also an argument, how to represent this? * @type Boolean */ - staticHeight: false, + get staticHeight() { + return this.args.staticHeight ?? false; + } /** * Indicates whether or not list items in the Radar should be reused on update of virtual components (e.g. scroll). @@ -245,10 +241,12 @@ const VerticalCollection = Component.extend({ * - When templates for individual items vary widely or are based on conditionals that are likely to change * (i.e. would defeat any benefits of DOM recycling anyway) * - * @property shouldRecycle + * @property shouldRecycle // TODO also an argument, how to represent this? * @type Boolean */ - shouldRecycle: true, + get shouldRecycle() { + return this.args.shouldRecycle ?? true; + } /* * A selector string that will select the element from @@ -260,8 +258,13 @@ const VerticalCollection = Component.extend({ * if so, you can leave this null. * * Set this to "body" to scroll the entire web page. + * + * @property containerSelector // TODO also an argument, how to represent this? + * @type String */ - containerSelector: '*', + get containerSelector() { + return this.args.containerSelector ?? '*'; + } // –––––––––––––– Performance Tuning /** @@ -269,11 +272,14 @@ const VerticalCollection = Component.extend({ * Increasing this value is useful when doing infinite scrolling and loading data from a remote service, * with the desire to allow records to show as the user scrolls and the backend API takes time to respond. * - * @property bufferSize + * @property bufferSize // TODO also an argument, how to represent this? * @type Number * @default 1 */ - bufferSize: 1, + @dependentKeyCompat + get bufferSize() { + return this.args.bufferSize ?? 1; + } // –––––––––––––– Initial Scroll State /** @@ -284,19 +290,22 @@ const VerticalCollection = Component.extend({ * * If the item cannot be found, scrollTop * is set to 0. - * @property idForFirstItem + * + * @argument idForFirstItem // TODO: How to represent this */ - idForFirstItem: null, /** * If set, if scrollPosition is empty * at initialization, the component will * render starting at the bottom. - * @property renderFromLast + * + * @property renderFromLast // TODO also an argument, how to represent this? * @type Boolean * @default false */ - renderFromLast: false, + get renderFromLast() { + return this.args.renderFromLast ?? false; + } /** * If set to true, the collection will render all of the items passed into the component. @@ -308,48 +317,70 @@ const VerticalCollection = Component.extend({ * - Can be used to respond to the keyboard input for Find (i.e. ctrl+F/cmd+F) to show all elements, which then * allows the list items to be searchable * - * @property renderAll + * @property renderAll // TODO also an argument, how to represent this? * @type Boolean * @default false */ - renderAll: false, + @dependentKeyCompat + get renderAll() { + return this.args.renderAll ?? false; + } /** * The tag name used in DOM elements before and after the rendered list. By default, it is set to * 'occluded-content' to avoid any confusion with user's CSS settings. However, it could be * overriden to provide custom behavior (for example, in table user wants to set it to 'tr' to * comply with table semantics). + * + * @property occlusionTagName // TODO also an argument, how to represent this? + * @type String + * @default occluded-content */ - occlusionTagName: 'occluded-content', + get occlusionTagName() { + return this.args.occlusionTagName ?? 'occluded-content'; + } - isEmpty: empty('items'), - shouldYieldToInverse: readOnly('isEmpty'), + get isEmpty() { + return !!(this.args.items?.length); + } - virtualComponents: computed('items.[]', 'renderAll', 'estimateHeight', 'bufferSize', function() { - const { _radar } = this; + get shouldYieldToInverse() { + return this.isEmpty; + } - const items = this.items; + // eslint-disable-next-line ember/use-brace-expansion + @computed('args.estimateHeight', 'args.items.[]', 'bufferSize', 'renderAll') + get virtualComponents() { + const { + _radar, + args: { + estimateHeight, + items + }, + bufferSize, + renderAll + } = this; _radar.items = items === null || items === undefined ? [] : items; - _radar.estimateHeight = this.estimateHeight; - _radar.renderAll = this.renderAll; - _radar.bufferSize = this.bufferSize; + _radar.estimateHeight = estimateHeight; + _radar.renderAll = renderAll; + _radar.bufferSize = bufferSize; _radar.scheduleUpdate(true); this._clearScheduledActions(); return _radar.virtualComponents; - }), + } schedule(queueName, job) { - return scheduler.schedule(queueName, job, this.token); - }, + scheduler.schedule(queueName, job, this.token); + } _clearScheduledActions() { clearTimeout(this._nextSendActions); this._nextSendActions = null; this._scheduledActions.length = 0; - }, + } _scheduleSendAction(action, index) { this._scheduledActions.push([action, index]); @@ -359,7 +390,7 @@ const VerticalCollection = Component.extend({ this._nextSendActions = null; run(() => { - const items = this.items; + const items = this.args.items; const keyPath = this.key; this._scheduledActions.forEach(([action, index]) => { @@ -367,18 +398,21 @@ const VerticalCollection = Component.extend({ const key = keyForItem(item, keyPath, index); // this.sendAction will be deprecated in ember 4.0 - const _action = get(this, action); + const _action = this.args[action]; if (typeof _action == 'function') { _action(item, index, key); } else if (typeof _action === 'string') { - this.sendAction(action, item, index, key); + /* + * This check and exception should be removed in Vertical Collection 6 + */ + throw new Error('vertical-collection no longer supports string based action') } }); this._scheduledActions.length = 0; }); }); } - }, + } /* Public API Methods @index => number @@ -396,15 +430,15 @@ const VerticalCollection = Component.extend({ return new Promise ((resolve) => { _radar.scheduleUpdate(false, resolve); }); - }, + } // –––––––––––––– Setup/Teardown - didInsertElement() { - this._super(); + @action + templateDidRender() { this.schedule('sync', () => { this._radar.start(); }); - }, + } willDestroy() { this.token.cancel(); @@ -422,27 +456,29 @@ const VerticalCollection = Component.extend({ this.__visualization = null; } } - this._super(); - }, + super.willDestroy(); + } - init() { - this._super(); + constructor(...args) { + super(...args); this.token = new Token(); const RadarClass = this.staticHeight ? StaticRadar : DynamicRadar; - const items = this.items || []; + const items = this.args.items || []; const { + args: { + estimateHeight, + idForFirstItem, + initialRenderCount + }, bufferSize, containerSelector, - estimateHeight, - initialRenderCount, renderAll, renderFromLast, shouldRecycle, occlusionTagName, - idForFirstItem, key } = this; @@ -473,10 +509,10 @@ const VerticalCollection = Component.extend({ this._scheduledActions = []; this._nextSendActions = null; - let a = !!this.lastReached; - let b = !!this.firstReached; - let c = !!this.lastVisibleChanged; - let d = !!this.firstVisibleChanged; + let a = !!this.args.lastReached; + let b = !!this.args.firstReached; + let c = !!this.args.lastVisibleChanged; + let d = !!this.args.firstVisibleChanged; let any = a || b || c || d; if (any) { @@ -590,7 +626,7 @@ const VerticalCollection = Component.extend({ assert(`itemContainer must define position`, styleIsOneOf(styles, 'position', ['static', 'relative', 'absolute'])); // check item defaults - assert(`You must supply at least one item to the collection to debug it's CSS.`, this.items.length); + assert(`You must supply at least one item to the collection to debug it's CSS.`, this.args.items.length); let element = radar._itemContainer.firstElementChild; @@ -601,7 +637,7 @@ const VerticalCollection = Component.extend({ }; } } -}); +} function calculateStartingIndex(items, idForFirstItem, key, renderFromLast) { const totalItems = get(items, 'length'); @@ -623,4 +659,6 @@ function calculateStartingIndex(items, idForFirstItem, key, renderFromLast) { return startingIndex; } +setComponentTemplate(layout, VerticalCollection); + export default VerticalCollection; diff --git a/addon/components/vertical-collection/template.hbs b/addon/components/vertical-collection/template.hbs index 55300b08..eab8c753 100644 --- a/addon/components/vertical-collection/template.hbs +++ b/addon/components/vertical-collection/template.hbs @@ -1,3 +1,4 @@ +{{vertical-collection/call this.templateDidRender}} {{#each this.virtualComponents key="id" as |virtualComponent| ~}} {{~unbound virtualComponent.upperBound~}} {{~#if virtualComponent.isOccludedContent ~}} diff --git a/addon/helpers/vertical-collection/call.js b/addon/helpers/vertical-collection/call.js new file mode 100644 index 00000000..6cd2a8fa --- /dev/null +++ b/addon/helpers/vertical-collection/call.js @@ -0,0 +1,5 @@ +import { helper } from '@ember/component/helper'; + +export default helper(function([fn]) { + fn(); +}); diff --git a/app/helpers/vertical-collection/call.js b/app/helpers/vertical-collection/call.js new file mode 100644 index 00000000..8893cff8 --- /dev/null +++ b/app/helpers/vertical-collection/call.js @@ -0,0 +1 @@ +export { default } from '@html-next/vertical-collection/helpers/vertical-collection/call'; diff --git a/package.json b/package.json index 2c5395e0..175311bb 100644 --- a/package.json +++ b/package.json @@ -57,6 +57,7 @@ "broccoli-funnel": "^3.0.8", "broccoli-merge-trees": "^4.2.0", "broccoli-rollup": "^5.0.0", + "ember-cached-decorator-polyfill": "^1.0.2", "ember-cli-babel": "^8.0.0", "ember-cli-htmlbars": "^6.3.0", "ember-cli-version-checker": "^5.1.2", diff --git a/tests/dummy/app/routes/acceptance-tests/record-array/template.hbs b/tests/dummy/app/routes/acceptance-tests/record-array/template.hbs index 84ddd8b0..79020990 100644 --- a/tests/dummy/app/routes/acceptance-tests/record-array/template.hbs +++ b/tests/dummy/app/routes/acceptance-tests/record-array/template.hbs @@ -5,7 +5,7 @@ @items={{this.items}} @estimateHeight={{50}} @bufferSize={{5}} - @firstVisibleChanged={{action "firstVisibleChanged"}} + @firstVisibleChanged={{this.firstVisibleChanged}} as |item index| > diff --git a/yarn.lock b/yarn.lock index 705ef76b..ab6b2709 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4525,7 +4525,7 @@ ember-cache-primitive-polyfill@^1.0.1: ember-compatibility-helpers "^1.2.1" silent-error "^1.1.1" -ember-cached-decorator-polyfill@^1.0.1: +ember-cached-decorator-polyfill@^1.0.1, ember-cached-decorator-polyfill@^1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/ember-cached-decorator-polyfill/-/ember-cached-decorator-polyfill-1.0.2.tgz#26445056ebee3776c340e28652ce59be73dd3958" integrity sha512-hUX6OYTKltAPAu8vsVZK02BfMTV0OUXrPqvRahYPhgS7D0I6joLjlskd7mhqJMcaXLywqceIy8/s+x8bxF8bpQ==