Skip to content

Commit

Permalink
DataGrid: load more and virtual scrolling (#2799)
Browse files Browse the repository at this point in the history
  • Loading branch information
wanghoppe authored and gingi committed Nov 9, 2023
1 parent 64a211f commit 4079c51
Show file tree
Hide file tree
Showing 29 changed files with 1,098 additions and 430 deletions.
2 changes: 1 addition & 1 deletion desktop/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion desktop/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@
"peerDependencies": {
"@fluentui/react": ">=8.97.2 <9.0.0",
"@fluentui/react-theme-provider": ">=0.19.16 <1.0.0",
"@uifabric/azure-themes": ">=7.5.19 <8.0.0",
"@fluentui/azure-themes": ">=8.6.34 < 9.0.0",
"mobx": "^6.3.2",
"mobx-react-lite": "^3.2.0",
"react": ">=17.0.2 <18.0.0",
Expand Down
390 changes: 118 additions & 272 deletions package-lock.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
"node": ">=18.0.0"
},
"dependencies": {
"@fluentui/azure-themes": "8.6.34",
"@fluentui/react": "8.97.2",
"@fluentui/react-theme-provider": "0.19.16",
"@uifabric/azure-themes": "7.5.19",
"mobx": "^6.3.2",
"mobx-react-lite": "^3.2.0",
"monaco-editor": "~0.31.0",
Expand Down
6 changes: 6 additions & 0 deletions packages/bonito-core/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,9 @@ export class UnexpectedStatusCodeError extends Error {
super(message);
}
}

export class CancelledPromiseError extends Error {
constructor(message: string) {
super(message);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { cancellablePromise } from "../cancellable-promise";

import { CancelledPromiseError } from "../../errors";

describe("Cancellable promise", () => {
test("should cancel promise properly", () => {
const p = Promise.resolve("resolved");
const cp = cancellablePromise(p);
cp.cancel();
return expect(cp).rejects.toThrowError(CancelledPromiseError);
});

test("should not cancel promise if it is already resolved or rejected", async () => {
const p = Promise.resolve("resolved");
const cp = cancellablePromise(p);
await expect(cp).resolves.toBe("resolved");
cp.cancel();
await expect(cp).resolves.toBe("resolved");

const p2 = Promise.reject("rejected");
const cp2 = cancellablePromise(p2);
await expect(cp2).rejects.toBe("rejected");
cp2.cancel();
await expect(cp2).rejects.toBe("rejected");
});

test("should override the promise's own rejection", async () => {
const p = Promise.reject("rejected");
const cp = cancellablePromise(p);
cp.cancel();
await expect(cp).rejects.toThrowError("Promise cancelled");
});
});
19 changes: 19 additions & 0 deletions packages/bonito-core/src/util/cancellable-promise.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { CancelledPromiseError } from "../errors";

export interface CancellablePromise<T> extends Promise<T> {
cancel: () => void;
}

export function cancellablePromise<T>(p: Promise<T>): CancellablePromise<T> {
let cancel: () => void = () => null;
const promise = new Promise<T>((resolve, reject) => {
p.then(resolve, reject);
cancel = () => {
reject(new CancelledPromiseError("Promise cancelled"));
};
}) as CancellablePromise<T>;

promise.cancel = cancel;

return promise;
}
1 change: 1 addition & 0 deletions packages/bonito-core/src/util/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ export * from "./functions";
export * from "./ordered-map";
export * from "./string";
export * from "./deferred";
export * from "./cancellable-promise";
1 change: 1 addition & 0 deletions packages/bonito-ui/i18n/resources.resjson
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"bonito.ui.dataGrid.noResults": "No results found",
"bonito.ui.form.buttons.apply": "Apply",
"bonito.ui.form.buttons.discardChanges": "Discard changes",
"bonito.ui.form.showPassword": "Show password"
Expand Down
2 changes: 1 addition & 1 deletion packages/bonito-ui/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/bonito-ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
"peerDependencies": {
"@fluentui/react": ">=8.97.2 <9.0.0",
"@fluentui/react-theme-provider": ">=0.16.2 <1.0.0",
"@uifabric/azure-themes": ">=7.5.19 <8.0.0",
"@fluentui/azure-themes": ">=8.6.34 < 9.0.0",
"mobx": "^6.3.2",
"mobx-react-lite": "^3.2.0",
"monaco-editor": ">=0.30.0 <0.40.0",
Expand Down
178 changes: 169 additions & 9 deletions packages/bonito-ui/src/components/__tests__/data-grid.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as React from "react";
import { runAxe } from "../../test-util/a11y";
import { initMockBrowserEnvironment } from "../../environment";
import { DataGrid } from "../data-grid";
import { fromIso } from "@azure/bonito-core";
import { fromIso, translate } from "@azure/bonito-core";

const ignoredA11yRules = {
rules: {
Expand All @@ -25,10 +25,12 @@ describe("DataGrid component", () => {
render(<DataGrid />);
const gridEl = screen.getByRole("grid");

// One row: the column header
expect(gridEl.getAttribute("aria-rowcount")).toBe("1");
expect(screen.getAllByRole("row").length).toBe(1);
expect(getOffsetAriaRowCount(gridEl)).toBe(0);
expect(getOffsetRowCount()).toBe(0);
expect(screen.getAllByRole("columnheader").length).toBe(1);
expect(
screen.getByText(translate("bonito.ui.dataGrid.noResults"))
).not.toBeNull();
});

test("Simple grid", async () => {
Expand All @@ -45,10 +47,9 @@ describe("DataGrid component", () => {
);
const gridEl = screen.getByRole("grid");

// Header plus 2 data rows, sorted alphabetically by the first column
const rows = screen.getAllByRole("row");
expect(gridEl.getAttribute("aria-rowcount")).toBe("3");
expect(rows.length).toBe(3);
expect(getOffsetAriaRowCount(gridEl)).toBe(2);
expect(getOffsetRowCount()).toBe(2);

const columnHeaders = screen.getAllByRole("columnheader");
// One extra column header for the select checkbox column
Expand Down Expand Up @@ -98,9 +99,9 @@ describe("DataGrid component", () => {
);
const gridEl = screen.getByRole("grid");

// Header plus 3 data rows
const rows = screen.getAllByRole("row");
expect(gridEl.getAttribute("aria-rowcount")).toBe("4");
// header + 3 data rows + footer
expect(gridEl.getAttribute("aria-rowcount")).toBe("5");
expect(screen.getAllByRole("row").length).toBe(4);

const columnHeaders = screen.getAllByRole("columnheader");
Expand Down Expand Up @@ -136,6 +137,118 @@ describe("DataGrid component", () => {

expect(await runAxe(container, ignoredA11yRules)).toHaveNoViolations();
});

test("Shimmer lines", async () => {
const { container, rerender } = render(
<DataGrid columns={["data"]} hasMore={true} items={[]} />
);
const gridEl = screen.getByRole("grid");

// 10 shimmer lines
expect(getOffsetAriaRowCount(gridEl)).toBe(10);
expect(getNumOfShimmerLines(container)).toBe(10);

rerender(
<DataGrid
columns={["data"]}
hasMore={true}
items={generateDataItems(3)}
/>
);

// 3 data rows + 3 shimmer lines
expect(getOffsetAriaRowCount(gridEl)).toBe(6);
expect(getNumOfShimmerLines(container)).toBe(3);

rerender(
<DataGrid
columns={["data"]}
hasMore={false}
items={generateDataItems(3)}
/>
);

// 3 data rows
expect(getOffsetAriaRowCount(gridEl)).toBe(3);
expect(getNumOfShimmerLines(container)).toBe(0);

expect(await runAxe(container, ignoredA11yRules)).toHaveNoViolations();
});

test("onLoadMore callback", async () => {
const onLoadMore = jest.fn();
const { container, rerender } = render(
<DataGrid hasMore={true} items={[]} onLoadMore={onLoadMore} />
);
// initial loading, should not trigger onLoadMore
expect(onLoadMore).not.toHaveBeenCalled();
rerender(
<DataGrid
hasMore={true}
items={[{ data: 1 }]}
onLoadMore={onLoadMore}
/>
);
// already has items, should trigger onLoadMore
expect(onLoadMore).toHaveBeenCalled();

expect(await runAxe(container, ignoredA11yRules)).toHaveNoViolations();
});

test("Virtual scrolling", async () => {
// DetailsList of FluentUI utilizes virtual scrolling when items is more
// than 10

// Max number of tries to wait for virtual scrolling to kick in
const maxTry = 10;
const onLoadMore = jest.fn();
let numTry = 0;
let expectedOnLoadMoreCount = 0;
let items = generateDataItems(1);

const renderGrid = () => (
<DataGrid
hasMore={true}
columns={["data"]}
items={items}
onLoadMore={onLoadMore}
/>
);

const { container, rerender } = render(renderGrid());
const gridEl = screen.getByRole("grid");

expect(getOffsetRowCount()).toBe(items.length);
// aria-rowcount should iclude 3 lines of shimmering
expect(getOffsetAriaRowCount(gridEl)).toBe(items.length + 3);

// getOffsetRowCount() === items.length means virtual scrolling is not
// working, retry until it works or maxTry is reached
while (getOffsetRowCount() === items.length) {
if (numTry++ > maxTry) {
throw new Error("Virtual scrolling is not working");
}
// If there is at least one shimmer line, onLoadMore should be
// triggered
if (getNumOfShimmerLines(container) >= 1) {
expectedOnLoadMoreCount++;
}

items = generateDataItems(items.length + 5);
rerender(renderGrid());
}
expect(onLoadMore).toHaveBeenCalledTimes(expectedOnLoadMoreCount);

// Virtual scrolling is working, the number of rows should be less than
// the number of items, and all shimmer lines should not in view
expect(getOffsetRowCount()).toBeLessThan(items.length);
expect(getNumOfShimmerLines(container)).toBe(0);

// aria-rowcount should iclude 3 lines of shimmering
expect(getOffsetAriaRowCount(gridEl)).toBe(items.length + 3);

expect(await runAxe(container, ignoredA11yRules)).toHaveNoViolations();
});
});

/**
Expand All @@ -162,3 +275,50 @@ function getColumnHeaderText(columnHeader: HTMLElement): string {
);
return el?.textContent ?? "";
}

/**
* Helper to get the shimmer lines in a data grid
* @param container The container element
* @returns The shimmer lines
*/
function getNumOfShimmerLines(container: HTMLElement): number {
return container.querySelectorAll(".ms-Shimmer-container").length;
}

/**
* Helper to get the value of aria-rowcount in a data grid, includes
* shimmer lines and doens't take into account the header and footer
* rows. The return number of rows is aria-rowcount - 2.
* @param gridEl The data grid element
* @returns The number of rows in the grid
*/
function getOffsetAriaRowCount(gridEl: HTMLElement): number {
const rowCount = gridEl.getAttribute("aria-rowcount");
if (!rowCount) {
throw new Error("aria-rowcount attribute not found");
}
// DataGrid's aria-rowcount is two more than the actual number of rows,
// one for the header and one for the footer.
return parseInt(rowCount, 10) - 2;
}

/**
* Helper to get the number of rows in a data grid, doens't take into
* account the header row or the shimmer lines, the return number of
* rows is the number of rows - 1.
*/
function getOffsetRowCount(): number {
return screen.getAllByRole("row").length - 1;
}

/**
* Helper to generate an array of data items
* @param num The number of items to generate
* @returns The array of data items
*/
function generateDataItems(num: number = 3): { data: number }[] {
const arr = Array(num)
.fill(0)
.map((_, i) => ({ data: i }));
return arr;
}
Loading

0 comments on commit 4079c51

Please sign in to comment.