Add loading spinner when filtering is in progress (#160)

* apply loading spinner when filtering is in progress * Change files * Update packages/components/src/test/tree/controlled/PresentationTreeRenderer.test.tsx Co-authored-by: Grigas <35135765+grigasp@users.noreply.github.com> * Update packages/components/src/test/tree/controlled/PresentationTreeRenderer.test.tsx Co-authored-by: Saulius Skliutas <24278440+saskliutas@users.noreply.github.com> * pr comment fixes * Update packages/components/src/test/tree/controlled/UseHierarchyLevelFiltering.test.ts Co-authored-by: Saulius Skliutas <24278440+saskliutas@users.noreply.github.com> * pr comment fixes * fix tests * remove useCallback --------- Co-authored-by: Grigas <35135765+grigasp@users.noreply.github.com> Co-authored-by: Saulius Skliutas <24278440+saskliutas@users.noreply.github.com>
iTwin · Jun 15, 2023 · 55e97cb · 55e97cb
1 parent edf1e47
commit 55e97cb
Show file tree

Hide file tree

Showing 11 changed files with 158 additions and 63 deletions.
diff --git a/change/@itwin-presentation-components-d449e36d-6979-485b-908c-83c5514462a6.json b/change/@itwin-presentation-components-d449e36d-6979-485b-908c-83c5514462a6.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "`InstanceFilterDialog`: Show loading spinner when filtering is in progress.",
+  "packageName": "@itwin/presentation-components",
+  "email": "124659623+simasjar@users.noreply.github.com",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/components/src/presentation-components/tree/DataProvider.ts b/packages/components/src/presentation-components/tree/DataProvider.ts
@@ -7,7 +7,6 @@
  */
 
 import memoize from "micro-memoize";
-import { PropertyRecord } from "@itwin/appui-abstract";
 import { DelayLoadedTreeNodeItem, PageOptions, TreeNodeItem } from "@itwin/components-react";
 import { IDisposable, Logger } from "@itwin/core-bentley";
 import { IModelConnection } from "@itwin/core-frontend";
@@ -33,8 +32,8 @@ import { translate } from "../common/Utils";
 import { PresentationComponentsLoggerCategory } from "../ComponentsLoggerCategory";
 import { convertToInstanceFilterDefinition } from "../instance-filter-builder/InstanceFilterConverter";
 import { IPresentationTreeDataProvider } from "./IPresentationTreeDataProvider";
-import { isPresentationTreeNodeItem, PresentationInfoTreeNodeItem, PresentationTreeNodeItem } from "./PresentationTreeNodeItem";
-import { createTreeNodeItem, CreateTreeNodeItemProps, pageOptionsUiToPresentation } from "./Utils";
+import { isPresentationTreeNodeItem, PresentationTreeNodeItem } from "./PresentationTreeNodeItem";
+import { createInfoNode, createTreeNodeItem, CreateTreeNodeItemProps, pageOptionsUiToPresentation } from "./Utils";
 
 /**
  * Properties for creating a `PresentationTreeDataProvider` instance.
@@ -341,17 +340,6 @@ function createTreeItems(
   return items;
 }
 
-function createInfoNode(parentNode: TreeNodeItem | undefined, message: string): PresentationInfoTreeNodeItem {
-  const id = parentNode ? `${parentNode.id}/info-node` : `/info-node/${message}`;
-  return {
-    id,
-    label: PropertyRecord.fromString(message),
-    message,
-    isSelectionDisabled: true,
-    children: undefined,
-  };
-}
-
 class MemoizationHelpers {
   public static areNodesRequestsEqual(
     lhsArgs: [TreeNodeItem?, PageOptions?, InstanceFilterDefinition?],

diff --git a/packages/components/src/presentation-components/tree/Utils.ts b/packages/components/src/presentation-components/tree/Utils.ts
@@ -8,12 +8,12 @@
 
 import { Observable as RxjsObservable } from "rxjs/internal/Observable";
 import { PropertyRecord } from "@itwin/appui-abstract";
-import { DelayLoadedTreeNodeItem, ItemColorOverrides, ItemStyle, Observable, PageOptions as UiPageOptions } from "@itwin/components-react";
+import { DelayLoadedTreeNodeItem, ItemColorOverrides, ItemStyle, Observable, TreeNodeItem, PageOptions as UiPageOptions } from "@itwin/components-react";
 import { CheckBoxState } from "@itwin/core-react";
 import { LabelDefinition, Node, NodeKey, PartialNode, PageOptions as PresentationPageOptions } from "@itwin/presentation-common";
 import { StyleHelper } from "../common/StyleHelper";
 import { createLabelRecord } from "../common/Utils";
-import { PresentationTreeNodeItem } from "./PresentationTreeNodeItem";
+import { PresentationInfoTreeNodeItem, PresentationTreeNodeItem } from "./PresentationTreeNodeItem";
 
 /** @internal */
 export interface CreateTreeNodeItemProps {
@@ -185,3 +185,14 @@ function createNodeLabelRecord(node: Node, appendChildrenCountForGroupingNodes:
 export function toRxjsObservable<T>(source: Observable<T>): RxjsObservable<T> {
   return new RxjsObservable((subscriber) => source.subscribe(subscriber));
 }
+
+export function createInfoNode(parentNode: TreeNodeItem | undefined, message: string): PresentationInfoTreeNodeItem {
+  const id = parentNode ? `${parentNode.id}/info-node` : `/info-node/${message}`;
+  return {
+    id,
+    label: PropertyRecord.fromString(message),
+    message,
+    isSelectionDisabled: true,
+    children: undefined,
+  };
+}
diff --git a/.../components/src/presentation-components/tree/controlled/PresentationTreeNodeRenderer.scss b/.../components/src/presentation-components/tree/controlled/PresentationTreeNodeRenderer.scss
@@ -3,10 +3,6 @@
 * See LICENSE.md in the project root for license terms and full copyright notice.
 *--------------------------------------------------------------------------------------------*/
 
-.presentation-components-info-node {
-  color: var(--iui-color-text-muted);
-}
-
 .presentation-components-node {
   .presentation-components-node-action-buttons {
     visibility: hidden;

diff --git a/...s/components/src/presentation-components/tree/controlled/PresentationTreeNodeRenderer.tsx b/...s/components/src/presentation-components/tree/controlled/PresentationTreeNodeRenderer.tsx
@@ -11,7 +11,7 @@ import classnames from "classnames";
 import { TreeNodeRenderer, TreeNodeRendererProps } from "@itwin/components-react";
 import { TreeNode } from "@itwin/core-react";
 import { SvgCloseSmall, SvgFilter, SvgFilterHollow } from "@itwin/itwinui-icons-react";
-import { ButtonGroup, IconButton } from "@itwin/itwinui-react";
+import { ButtonGroup, IconButton, Text } from "@itwin/itwinui-react";
 import { isPresentationInfoTreeNodeItem, isPresentationTreeNodeItem, PresentationTreeNodeItem } from "../PresentationTreeNodeItem";
 
 /**
@@ -34,7 +34,7 @@ export function PresentationTreeNodeRenderer(props: PresentationTreeNodeRenderer
   const nodeItem = props.node.item;
 
   if (isPresentationInfoTreeNodeItem(nodeItem)) {
-    return <TreeNode className="presentation-components-info-node" isLeaf={true} label={nodeItem.message} level={props.node.depth} isHoverDisabled={true} />;
+    return <TreeNode isLeaf={true} label={<Text isMuted>{nodeItem.message}</Text>} level={props.node.depth} isHoverDisabled={true} />;
   }
 
   if (isPresentationTreeNodeItem(nodeItem)) {

diff --git a/...ages/components/src/presentation-components/tree/controlled/UseHierarchyLevelFiltering.ts b/...ages/components/src/presentation-components/tree/controlled/UseHierarchyLevelFiltering.ts
@@ -6,8 +6,8 @@
  * @module Tree
  */
 
-import { isTreeModelNode, ITreeNodeLoader, TreeModelSource, TreeNodeItem } from "@itwin/components-react";
-import { useCallback } from "react";
+import { isTreeModelNode, ITreeNodeLoader, Subscription, TreeModelSource, TreeNodeItem } from "@itwin/components-react";
+import { useRef } from "react";
 import { PresentationInstanceFilterInfo } from "../../instance-filter-builder/PresentationInstanceFilterBuilder";
 import { isPresentationTreeNodeItem } from "../PresentationTreeNodeItem";
 
@@ -27,25 +27,26 @@ export interface UseHierarchyLevelFilteringProps {
  */
 export function useHierarchyLevelFiltering(props: UseHierarchyLevelFilteringProps) {
   const { nodeLoader, modelSource } = props;
+  const ongoingSubscriptions = useRef(new Map<string, Subscription>());
 
-  const applyFilter = useCallback(
-    (node: TreeNodeItem, info: PresentationInstanceFilterInfo) => {
-      applyHierarchyLevelFilter(nodeLoader, modelSource, node.id, info);
-    },
-    [nodeLoader, modelSource],
-  );
-
-  const clearFilter = useCallback(
-    (node: TreeNodeItem) => {
-      applyHierarchyLevelFilter(nodeLoader, modelSource, node.id);
-    },
-    [nodeLoader, modelSource],
-  );
+  const handleFilterAction = (nodeId: string, info?: PresentationInstanceFilterInfo) => {
+    if (ongoingSubscriptions.current.has(nodeId)) {
+      ongoingSubscriptions.current.get(nodeId)!.unsubscribe();
+      ongoingSubscriptions.current.delete(nodeId);
+    }
+    const subscription = applyHierarchyLevelFilter(nodeLoader, modelSource, nodeId, () => ongoingSubscriptions.current.delete(nodeId), info);
+    if (subscription) {
+      ongoingSubscriptions.current.set(nodeId, subscription);
+    }
+  }
 
-  return { applyFilter, clearFilter };
+  return {
+    applyFilter: (node: TreeNodeItem, info: PresentationInstanceFilterInfo) => handleFilterAction(node.id, info),
+    clearFilter: (node: TreeNodeItem) => handleFilterAction(node.id),
+  };
 }
 
-function applyHierarchyLevelFilter(nodeLoader: ITreeNodeLoader, modelSource: TreeModelSource, nodeId: string, filter?: PresentationInstanceFilterInfo) {
+function applyHierarchyLevelFilter(nodeLoader: ITreeNodeLoader, modelSource: TreeModelSource, nodeId: string, onComplete: (id: string) => void, filter?: PresentationInstanceFilterInfo) {
   modelSource.modifyModel((model) => {
     const modelNode = model.getNode(nodeId);
     if (!modelNode || !isTreeModelNode(modelNode) || !isPresentationTreeNodeItem(modelNode.item) || !modelNode.item.filtering) {
@@ -55,6 +56,7 @@ function applyHierarchyLevelFilter(nodeLoader: ITreeNodeLoader, modelSource: Tre
     modelNode.item.filtering.active = filter;
     if (filter) {
       modelNode.isExpanded = true;
+      modelNode.isLoading = true;
     }
     model.clearChildren(nodeId);
   });
@@ -63,5 +65,5 @@ function applyHierarchyLevelFilter(nodeLoader: ITreeNodeLoader, modelSource: Tre
   if (updatedNode === undefined || !updatedNode.isExpanded || updatedNode.numChildren !== undefined) {
     return;
   }
-  nodeLoader.loadNode(updatedNode, 0).subscribe();
+  return nodeLoader.loadNode(updatedNode, 0).subscribe({ complete: () => onComplete(nodeId), error: () => onComplete(nodeId) });
 }
diff --git a/packages/components/src/test/tree/controlled/Helpers.ts b/packages/components/src/test/tree/controlled/Helpers.ts
@@ -21,13 +21,13 @@ export function createTreeNodeItem(item?: Partial<PresentationTreeNodeItem>): Pr
 export function createTreeModelNode(node?: Partial<TreeModelNode>, nodeItem?: TreeNodeItem): TreeModelNode {
   const label = nodeItem?.label ?? node?.label ?? PropertyRecord.fromString("TestLabel");
   return {
-    id: node?.id ?? "0",
-    parentId: node?.parentId,
+    id: node?.id ?? nodeItem?.id ?? "0",
+    parentId: nodeItem?.parentId ?? node?.parentId,
     numChildren: node?.numChildren,
     depth: node?.depth ?? 0,
     isExpanded: node?.isExpanded ?? false,
     isSelected: node?.isSelected ?? false,
-    description: node?.description ?? "Node Description",
+    description: nodeItem?.description ?? node?.description ?? "Node Description",
     checkbox: node?.checkbox ?? {
       isDisabled: false,
       isVisible: true,

diff --git a/packages/components/src/test/tree/controlled/PresentationTreeRenderer.test.tsx b/packages/components/src/test/tree/controlled/PresentationTreeRenderer.test.tsx
@@ -7,16 +7,17 @@ import { expect } from "chai";
 import sinon from "sinon";
 import * as moq from "typemoq";
 import { PropertyRecord, StandardTypeNames } from "@itwin/appui-abstract";
-import { ITreeNodeLoader, TreeActions, TreeModel, TreeModelSource, UiComponents, VisibleTreeNodes } from "@itwin/components-react";
+import { ITreeNodeLoader, TreeActions, TreeModel, TreeModelSource, TreeNodeLoadResult, UiComponents, VisibleTreeNodes } from "@itwin/components-react";
 import { EmptyLocalization } from "@itwin/core-common";
 import { IModelApp, IModelConnection } from "@itwin/core-frontend";
 import { PropertyValueFormat } from "@itwin/presentation-common";
 import { Presentation } from "@itwin/presentation-frontend";
-import { fireEvent, render, waitFor } from "@testing-library/react";
+import { fireEvent, render, RenderResult, waitFor } from "@testing-library/react";
 import { PresentationTreeRenderer, PresentationTreeRendererProps } from "../../../presentation-components/tree/controlled/PresentationTreeRenderer";
 import { createTestPropertyInfo, stubDOMMatrix, stubRaf } from "../../_helpers/Common";
 import { createTestContentDescriptor, createTestPropertiesContentField,  } from "../../_helpers/Content";
 import { createTreeModelNode, createTreeNodeItem } from "./Helpers";
+import { Subject } from "rxjs";
 
 describe("PresentationTreeRenderer", () => {
   stubRaf();
@@ -118,11 +119,55 @@ describe("PresentationTreeRenderer", () => {
     modelSourceMock.setup((x) => x.getModel()).returns(() => treeModelMock.object);
     modelSourceMock.setup((x) => x.modifyModel(moq.It.isAny())).verifiable(moq.Times.once());
 
-    const { getByText, container, baseElement } = render(<PresentationTreeRenderer {...treeProps} />);
+    const result = render(<PresentationTreeRenderer {...treeProps} />);
 
+    const { getByText, baseElement } = result
     await waitFor(() => getByText(label));
 
-    const filterButton = container.querySelector(".presentation-components-node-action-buttons button");
+    await applyFilter(result, propertyField.label);
+
+    // wait until dialog closes
+    await waitFor(() => {
+      expect(baseElement.querySelector(".presentation-instance-filter-dialog")).to.be.null;
+    });
+
+    modelSourceMock.verifyAll();
+  });
+
+  it("sets `node.isLoading` to true when filter is applied", async () => {
+    const subject = new Subject<TreeNodeLoadResult>();
+    nodeLoaderMock.setup((x) => x.loadNode(moq.It.isAny(), 0)).returns(() => subject);
+    const label = "Node Label";
+    const property = createTestPropertyInfo({ name: "TestProperty", type: StandardTypeNames.Bool });
+    const propertyField = createTestPropertiesContentField({
+      properties: [{ property }],
+      name: property.name,
+      label: property.name,
+      type: { typeName: StandardTypeNames.Bool, valueFormat: PropertyValueFormat.Primitive },
+    });
+    const nodeItem = createTreeNodeItem({ filtering: { descriptor: createTestContentDescriptor({ fields: [propertyField] }) } });
+    const node = createTreeModelNode({ label: PropertyRecord.fromString(label), isExpanded: true }, nodeItem);
+    const modelSource = new TreeModelSource();
+    modelSource.modifyModel((model) => {
+      model.setChildren(undefined, [{...node, isLoading: false}], 0);
+    });
+    visibleNodesMock.setup((x) => x.getNumNodes()).returns(() => 1);
+    visibleNodesMock.setup((x) => x.getAtIndex(0)).returns(() => modelSource.getModel().getNode(node.id));
+
+    const result = render(<PresentationTreeRenderer {...treeProps} modelSource={modelSource} />);
+    const { getByText } = result;
+    await waitFor(() => getByText(label));
+
+    await applyFilter(result, propertyField.label);
+    const modelSourceNode = modelSource.getModel().getNode(node.id)!;
+    expect(modelSourceNode.isLoading).to.be.true;
+    subject.complete();
+    nodeLoaderMock.verify((x) => x.loadNode(modelSourceNode, 0), moq.Times.once());
+  });
+});
+
+async function applyFilter({ container, baseElement, getByText }: RenderResult, propertyLabel: string) {
+  const filterButton = container.querySelector(".presentation-components-node-action-buttons button");
     expect(filterButton).to.not.be.null;
     fireEvent.click(filterButton!);
 
@@ -137,7 +182,7 @@ describe("PresentationTreeRenderer", () => {
     expect(propertySelector).to.not.be.null;
     fireEvent.focus(propertySelector!);
     // select property
-    fireEvent.click(getByText(propertyField.label));
+    fireEvent.click(getByText(propertyLabel));
 
     // wait until apply button is enabled
     const applyButton = await waitFor(() => {
@@ -146,12 +191,4 @@ describe("PresentationTreeRenderer", () => {
       return button;
     });
     fireEvent.click(applyButton!);
-
-    // wait until dialog closes
-    await waitFor(() => {
-      expect(baseElement.querySelector(".presentation-instance-filter-dialog")).to.be.null;
-    });
-
-    modelSourceMock.verifyAll();
-  });
-});
+}
diff --git a/packages/components/src/test/tree/controlled/UseHierarchyLevelFiltering.test.ts b/packages/components/src/test/tree/controlled/UseHierarchyLevelFiltering.test.ts
@@ -7,7 +7,7 @@ import { expect } from "chai";
 import { from } from "rxjs/internal/observable/from";
 import * as moq from "typemoq";
 import { PropertyRecord } from "@itwin/appui-abstract";
-import { ITreeNodeLoader, PropertyFilterRuleOperator, TreeModelNodeInput, TreeModelSource, UiComponents } from "@itwin/components-react";
+import { ITreeNodeLoader, PropertyFilterRuleOperator, TreeModelNodeInput, TreeModelSource, TreeNodeLoadResult, UiComponents } from "@itwin/components-react";
 import { EmptyLocalization } from "@itwin/core-common";
 import { renderHook } from "@testing-library/react-hooks";
 import { PresentationInstanceFilterInfo } from "../../../presentation-components/instance-filter-builder/PresentationInstanceFilterBuilder";
@@ -16,6 +16,7 @@ import { PresentationTreeNodeItem } from "../../../presentation-components/tree/
 import { createTestPropertyInfo } from "../../_helpers/Common";
 import { createTestContentDescriptor, createTestPropertiesContentField } from "../../_helpers/Content";
 import { createTestECInstancesNodeKey } from "../../_helpers/Hierarchy";
+import { Subject } from "rxjs";
 
 function createTreeModelInput(input?: Partial<TreeModelNodeInput>, treeItem?: Partial<PresentationTreeNodeItem>): TreeModelNodeInput {
   const item: PresentationTreeNodeItem = {
@@ -208,4 +209,57 @@ describe("useHierarchyLevelFiltering", () => {
     result.current.clearFilter(parentNode.item);
     expect(modelSource.getModel().getNode(childNode.id)).to.be.undefined;
   });
+
+  it("clears filter unsubscribes from observable created by `applyFilter`", () => {
+    const applyFilterActionSubject = new Subject<TreeNodeLoadResult>();
+    const clearFilterActionSubject = new Subject<TreeNodeLoadResult>();
+    nodeLoaderMock.setup((x) => x.loadNode(moq.It.isAny(), 0)).returns(() => applyFilterActionSubject);
+    nodeLoaderMock.setup((x) => x.loadNode(moq.It.isAny(), 0)).returns(() => clearFilterActionSubject);
+
+    const node = createTreeModelInput(undefined, { filtering: { descriptor: createTestContentDescriptor({ fields: [] }) } });
+    modelSource.modifyModel((model) => {
+      model.setChildren(undefined, [node], 0);
+    });
+
+    const { result } = renderHook(useHierarchyLevelFiltering, { initialProps: { modelSource, nodeLoader: nodeLoaderMock.object } });
+
+    result.current.applyFilter(node.item, filterInfo);
+    expect(applyFilterActionSubject.observers.length).to.be.eq(1);
+    result.current.clearFilter(node.item);
+
+    expect(applyFilterActionSubject.observers.length).to.be.eq(0);
+  });
+
+  it("`applyFilter` unsubscribes from previous observable if called second time", () => {
+    const subject = new Subject<TreeNodeLoadResult>();
+    nodeLoaderMock.setup((x) => x.loadNode(moq.It.isAny(), 0)).returns(() => subject);
+    const node = createTreeModelInput(undefined, { filtering: { descriptor: createTestContentDescriptor({ fields: [] }) } });
+    modelSource.modifyModel((model) => {
+      model.setChildren(undefined, [node], 0);
+    });
+
+    const { result } = renderHook(useHierarchyLevelFiltering, { initialProps: { modelSource, nodeLoader: nodeLoaderMock.object } });
+
+    result.current.applyFilter(node.item, filterInfo);
+    const firstObservable = subject.observers[0];
+    result.current.applyFilter(node.item, filterInfo);
+    expect(subject.observers.length).to.be.eq(1);
+    expect(firstObservable.closed).to.be.true;
+  });
+
+  it("unsubscribes from observable if error is thrown", () => {
+    const subject = new Subject<TreeNodeLoadResult>();
+    nodeLoaderMock.setup((x) => x.loadNode(moq.It.isAny(), 0)).returns(() => subject);
+    const node = createTreeModelInput(undefined, { filtering: { descriptor: createTestContentDescriptor({ fields: [] }) } });
+    modelSource.modifyModel((model) => {
+      model.setChildren(undefined, [node], 0);
+    });
+
+    const { result } = renderHook(useHierarchyLevelFiltering, { initialProps: { modelSource, nodeLoader: nodeLoaderMock.object } });
+
+    result.current.applyFilter(node.item, filterInfo);
+    expect(subject.observers.length).to.be.eq(1);
+    subject.error([]);
+    expect(subject.observers.length).to.be.eq(0);
+  });
 });
diff --git a/packages/full-stack-tests/src/components/tree/ErrorHandling.test.tsx b/packages/full-stack-tests/src/components/tree/ErrorHandling.test.tsx
@@ -77,7 +77,7 @@ describe("Learning snippets", () => {
       });
 
       // render the component
-      const { container, rerender } = render(<MyTree imodel={imodel} />);
+      const { container, getByText, rerender } = render(<MyTree imodel={imodel} />);
       await waitFor(() => getByRole(container, "tree"));
 
       // find & expand model A node
@@ -96,7 +96,7 @@ describe("Learning snippets", () => {
       toggleExpandNode(modelNodeB);
 
       // expect B model to have a single error node
-      await waitFor(() => expect(container.querySelector(".presentation-components-info-node")).is.not.null);
+      await waitFor(() => expect(getByText("Èrrór ¢rëätíñg thë hìérärçhý lévêl")).is.not.null);
       expect(() => getNodeByLabel(container, `B element 1`)).to.throw();
       expect(() => getNodeByLabel(container, `B element 2`)).to.throw();
 
@@ -105,7 +105,7 @@ describe("Learning snippets", () => {
       await waitFor(() => getByRole(container, "tree"));
       expect(() => getNodeByLabel(container, `My Model A`)).to.throw();
       expect(() => getNodeByLabel(container, `My Model B`)).to.throw();
-      expect(container.querySelector(".presentation-components-info-node")).is.not.null;
+      expect(getByText("Èrrór ¢rëätíñg thë hìérärçhý lévêl")).is.not.null;
     });
   });
 });