From a8a48a4ca42c6f5fdc59a48e0199939069e5614a Mon Sep 17 00:00:00 2001
From: David Boho <DavidWiesner@users.noreply.github.com>
Date: Tue, 26 Mar 2019 21:17:03 +0100
Subject: [PATCH] fix(virtual-scroll): use correct item top calculation with
 header or footer function (#15948) (#17345)

- use the right index in updateVDom to update the top transition ()
- extend unit test to verify the top is also calculated right with a given headerFn and footerFn
- update the visibility of the node also if a given approxHeaderHeight/approxFooterHeight matches the calculated height

fixes #15948 fixes #17298
---
 .../test/virtual-scroll-utils.spec.ts             | 15 ++++++++++-----
 .../virtual-scroll/virtual-scroll-utils.ts        |  2 +-
 .../components/virtual-scroll/virtual-scroll.tsx  |  6 +++---
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/core/src/components/virtual-scroll/test/virtual-scroll-utils.spec.ts b/core/src/components/virtual-scroll/test/virtual-scroll-utils.spec.ts
index 36efc1c9f5..03d08fb1d2 100644
--- a/core/src/components/virtual-scroll/test/virtual-scroll-utils.spec.ts
+++ b/core/src/components/virtual-scroll/test/virtual-scroll-utils.spec.ts
@@ -368,15 +368,20 @@ describe('updateVDom', () => {
   it('should initialize empty VDOM', () => {
     const vdom: VirtualNode[] = [];
     const items = [1, 2, 3, 4, 5];
-    const { heightIndex, cells } = mockVirtualScroll(items, () => 20);
-    const range: Range = { offset: 1, length: 4 };
+    const { heightIndex, cells } = mockVirtualScroll(items, () => 20,
+      (_, i) => i === 1 ? 'hola' : null,
+      (_, i) => i === 2 ? 'hola' : null
+    );
+    const range: Range = { offset: 1, length: 6 };
 
     updateVDom(vdom, heightIndex, cells, range);
     expect(vdom).toEqual([
       { cell: cells[1], change: 2, d: false, top: 20, visible: true },
-      { cell: cells[2], change: 2, d: false, top: 40, visible: true },
-      { cell: cells[3], change: 2, d: false, top: 60, visible: true },
-      { cell: cells[4], change: 2, d: false, top: 80, visible: true }
+      { cell: cells[2], change: 2, d: false, top: 30, visible: true },
+      { cell: cells[3], change: 2, d: false, top: 50, visible: true },
+      { cell: cells[4], change: 2, d: false, top: 70, visible: true },
+      { cell: cells[5], change: 2, d: false, top: 80, visible: true },
+      { cell: cells[6], change: 2, d: false, top: 100, visible: true }
     ]);
   });
 
diff --git a/core/src/components/virtual-scroll/virtual-scroll-utils.ts b/core/src/components/virtual-scroll/virtual-scroll-utils.ts
index 2324cbbfb3..3a663becee 100644
--- a/core/src/components/virtual-scroll/virtual-scroll-utils.ts
+++ b/core/src/components/virtual-scroll/virtual-scroll-utils.ts
@@ -46,7 +46,7 @@ export function updateVDom(dom: VirtualNode[], heightIndex: Uint32Array, cells:
 
   for (const cell of toMutate) {
     const node = pool.find(n => n.d && n.cell.type === cell.type);
-    const index = cell.index;
+    const index = cell.i;
     if (node) {
       node.d = false;
       node.change = NODE_CHANGE_CELL;
diff --git a/core/src/components/virtual-scroll/virtual-scroll.tsx b/core/src/components/virtual-scroll/virtual-scroll.tsx
index 76d3b2ae34..e8b07adfd3 100644
--- a/core/src/components/virtual-scroll/virtual-scroll.tsx
+++ b/core/src/components/virtual-scroll/virtual-scroll.tsx
@@ -327,9 +327,9 @@ export class VirtualScroll implements ComponentInterface {
     if (cell !== this.cells[index]) {
       return;
     }
-    cell.visible = true;
-    if (cell.height !== height) {
-      console.debug(`[virtual] cell height changed ${cell.height}px -> ${height}px`);
+    if (cell.height !== height || cell.visible !== true) {
+      console.debug(`[virtual] cell height or visibility changed ${cell.height}px -> ${height}px`);
+      cell.visible = true;
       cell.height = height;
       this.indexDirty = Math.min(this.indexDirty, index);
       this.scheduleUpdate();