virtualscroll by versusvirus · Pull Request #1 · StrikeX/listview

versusvirus · 2019-12-19T19:18:20Z

No description provided.

virtualscroll/controller.ts

StrikeX · 2020-01-16T18:37:20Z

virtualscroll/controller.ts

+    // TODO в этом методе нужно рассчитывать необходимо ли производить догрузку данных
+}
+
+export function recalcFromScrollTop(heights: IHeights): IRange {


вот это вообще нелогично.
пересчитать от скроллтопа - а передается вся латуха непонятно чего

StrikeX · 2020-01-16T18:38:51Z

virtualscroll/controller.ts

+    }
+}
+
+export function recalcFromItemHeightProperty(index: number, heights: IHeights): IRange {


тут тоже ничего не понятно
ты уверен, что нужен весь IHeight?
и что такое index?

StrikeX · 2020-01-16T18:40:36Z

virtualscroll/controller.ts

+    }
+}
+
+export function recalcToDirection(direction: IDirection, range: IRange, segmentSize: number, heights: IHeights): IRange {


здесь точно нужен весь IHeight?

у виртуального скролла нет вообще никакого состояния?
есть же какая то инициализация
а потом его начинают менять
разве не так?

StrikeX · 2020-01-16T18:45:19Z

scroll.ts

+            if (scrollUtils.canScrollToItem(getIndexById(id), this.range, this.heights)) {
+                scrollToElement(element);
+            } else {
+                await this.applyIndexes(VS.recalcFromIndex(getIndexById(id)));


а почему scroll вообще должен зависеть от виртуального скроллинга?
кто вызывает ScrollToItem?
когда?
что если он научиться возвращать результат, и вернет false
если не может проскроллить
то кто тогда мог бы сдвигать виртуальный скролл?

1.Потому что могут вызвать скролл к имеющейся, но не отрисованной записи
2.Прикладники, scrollViewer, календарь и прочие
3.Когда нужно прибить элемент к верхушке вьюпорта
4.Возможно стоит это сделать

StrikeX · 2020-01-16T18:52:18Z

scroll.ts

+class Scroll {
+    protected _beforeMount(options): void {
+        if (options.itemHeightProperty) {
+            this.applyIndexes(VS.recalcFromItemHeightProperty(options.activeElement, this.heights));


а что с this.heights? это одна из самых интересных штук
когда она меняется? и как?

интересно, почему называется recalc. если это первый calc?

StrikeX · 2020-01-16T18:53:07Z

scroll.ts

+
+class Scroll {
+    protected _beforeMount(options): void {
+        if (options.itemHeightProperty) {


это что?

и почему вилка такая? почему способ расчета виртуального скролла зависит от параметра конструктора?

Это нужно для отрисовки по заранее вычисленным высотам

StrikeX · 2020-01-16T19:02:09Z

virtualscroll/controller.ts

+    }
+}
+
+export function recalcToDirection(direction: IDirection, range: IRange, segmentSize: number, heights: IHeights): IRange {


у виртуального скролла нет вообще никакого состояния?
есть же какая то инициализация
а потом его начинают менять
разве не так?

StrikeX · 2020-01-16T19:03:30Z

scroll.ts

+    }
+
+    private recalcToDirection(triggerName): void {
+        this.applyIndexes(VS.recalcToDirection(triggerName, this.range, this._options.segmentSize, this.heights));


ты уверен, что в этом классе нужен this.range?

я ниже писал, точно ли не нужен никакой класс
вроде как, если был бы класс -то диапазон мог бы храниться в нем

StrikeX · 2020-01-17T18:53:22Z

virtualscroll/controller.ts

@@ -0,0 +1,163 @@
+interface IHeights {


подробно опиши что это такое и зачем

StrikeX · 2020-01-20T07:32:37Z

virtualscroll/interfaces.ts

+    // Высота вьюпорта
+    viewport: number;
+    // Высота позиции скролла
+    scrollTop: number;


убери от сюда scrollTop

StrikeX · 2020-01-20T07:32:54Z

virtualscroll/controller.ts

+     * @remark
+     * Вызывается при смещении скролла за счет движения скроллбара
+     */
+    updateRangeByScrollTop(): IRange {


передай сюда scrollTop

scroll.ts

StrikeX · 2020-01-22T07:15:16Z

scroll.ts

+     */
+    private _scrollMove(params: IScrollEventParams): void {
+        this._scrollTop = params.scrollTop;
+        const activeElementIndex = scrollUtils.getActiveElementIndex(


что это такое?

я думал, что при scrollMove нужно менять диапазон. а где это?

Не нужно, это не движения scrollBar.

Это высчитывание активного элемента(чтобы маркер в карточке контрагента при скролле передвигался)

StrikeX · 2020-01-22T07:18:36Z

scroll.ts

+        if (triggerState && this._options.virtualScroll) {
+            this._recalcToDirection(triggerName);
+        } else {
+            this._notifyLoadMore(triggerName);


вот тут опять нелогично. 3 подряд _notifyLoadMore
почему бы не сделать так, чтобы виртуальный скролл был всегда
просто размер виртуального окна сделать по всей высоте
тогда не нужно будет расставлять кучу условий

Возможно эта идея неплохая

StrikeX · 2020-02-06T13:48:55Z

scroll.ts

+     * @param direction
+     * @private
+     */
+    private _checkEdgeReached(direction: IDirection): boolean {


эту логику нужно перенести в virtualScroll
спрашивать его, где находится виртуальное окно: сверху или снизу?

StrikeX · 2020-02-06T13:51:22Z

virtualscroll/controller.ts

+        return this._range;
+    }
+
+    get itemsHeightsData() {


Зачем передавать itemsHeightsData а потом получать его?
он же никак не модифицируется и не рассчитывается
в итоге в месте вызова непонятная лишняя связь между модулями

StrikeX · 2020-02-06T13:56:00Z

virtualscroll/controller.ts

+        }
+    }
+
+    getPlaceholders(): IPlaceholders {


вот это нужно объединять с IRange

StrikeX · 2020-02-06T14:00:28Z

scroll.ts

+     * @param triggerName
+     * @param triggerState
+     */
+    private _triggerVisibilityChanged(triggerName: IDirection, triggerState: boolean): void {


что за state?

StrikeX · 2020-02-06T14:05:10Z

scroll.ts

+        if (index) {
+            return new Promise((resolve, reject) => {
+                if (this._virtualScroll.canScrollToItem(index)) {
+                    this._scrollToPosition(this._virtualScroll.itemsOffsets[index]);


убери itemsOffests

StrikeX · 2020-02-06T14:15:14Z

virtualscroll/controller.ts

+        let {start, stop} = this._range;
+
+        if (segmentSize) {
+            const quantity = VirtualScroll.getItemsToHideQuantity(direction, this._range, this._containerHeightsData, itemsHeightsData);


переделать в приватный метод

StrikeX · 2020-02-06T14:39:14Z

scroll.ts

+     * Вычисляет реальные высоты элемента
+     * @param container
+     */
+    private static getItemsHeightsDataByContainer(container: HTMLElement): IItemsHeights {


убрать вычисления в virtualScroll
добавить проверку на range

StrikeX · 2020-02-06T14:42:56Z

scroll.ts

+     */
+    private _viewportResize(params: IScrollEventParams): void {
+        this._virtualScroll.resizeViewport(params.viewportHeight, Scroll.getItemsHeightsDataByContainer(this._children.itemsContainer));
+        this._updateTriggerOffset(params.scrollHeight, params.viewportHeight);


внутри меняется virtualScroll
почему не сделать это за 1 раз?

StrikeX · 2020-02-07T20:11:27Z

scroll.ts

+    }
+
+    protected _afterMount(): void {
+        this.__mounted = true;


не понимаю зачем это
неужели какой-то код начинает выполняться раньше, чем этот флаг?

StrikeX · 2020-02-07T20:12:09Z

scroll.ts

+    protected _afterRender(): void {
+        if (this._virtualScroll.rangeChanged) {
+            this._virtualScroll.updateItemsHeights(this._children.itemsContainer);
+            this._virtualScroll.rangeChanged = false;


ты сказал, что удалишь этот флаг

StrikeX · 2020-02-07T20:15:13Z

scroll.ts

+    private _viewportResize(params: IScrollEventParams): void {
+        this._updateTriggerOffset(params.scrollHeight, params.viewportHeight);
+        this._virtualScroll.resizeViewport(params.viewportHeight, this._triggerOffset);
+        this._virtualScroll.updateItemsHeights(this._children.itemsContainer);


ты хотел сделать одним методом

StrikeX · 2020-02-07T20:15:22Z

scroll.ts

+    private _viewResize(params: IScrollEventParams): void {
+        this._updateTriggerOffset(params.scrollHeight, params.viewportHeight);
+        this._virtualScroll.resizeView(params.scrollHeight, this._triggerOffset);
+        this._virtualScroll.updateItemsHeights(this._children.itemsContainer);


туда же

virtualscroll

b1f862a

StrikeX reviewed Jan 16, 2020

View reviewed changes

virtualscroll/controller.ts Outdated Show resolved Hide resolved

StrikeX reviewed Jan 16, 2020

View reviewed changes

vd.volockoy added 16 commits January 17, 2020 14:15

virtualscroll

4492a67

virtualscroll

ae45c7e

virtualscroll

dc08742

virtualscroll

b114997

virtualscroll

977943a

virtualscroll

f469f46

virtualscroll

2bfaa71

virtualscroll

6362cfd

virtualscroll

fb9e679

virtualscroll

de71ead

virtualscroll

3ca54a8

virtualscroll

909ed3b

virtualscroll

6e0baef

virtualscroll

d8930dd

virtualscroll

d1fd8c5

virtualscroll

53c39a6

StrikeX reviewed Jan 23, 2020

View reviewed changes

vd.volockoy added 2 commits January 23, 2020 16:48

virtualscroll

ac1c46e

virtualscroll

b55ae8f

versusvirus force-pushed the vvd/virtualscroll branch from e5ced54 to b55ae8f Compare January 27, 2020 08:41

vd.volockoy added 2 commits February 3, 2020 18:03

virtualscroll

1405fe0

virtualscroll

66df10f

vd.volockoy added 5 commits February 4, 2020 13:41

virtualscroll

81165e8

virtualscroll

dd928f7

virtualscroll

b6705bc

virtualscroll

4503960

virtualscroll

36522a3

StrikeX reviewed Feb 7, 2020

View reviewed changes

virtualscroll

bdebad3

StrikeX reviewed Feb 7, 2020

View reviewed changes

Comments

Conversation

versusvirus commented Dec 19, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects