Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/saved-views-react/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,4 @@ export { useSavedViewTileContext, type SavedViewTileContext } from "./SavedViewT
export { StickyExpandableBlock } from "./StickyExpandableBlock/StickyExpandableBlock.js";
export { TileGrid } from "./TileGrid/TileGrid.js";
export { useSavedViews, type SavedViewsActions } from "./useSavedViews.js";
export { getActiveView, initViewOperations, listSavedViews, setView, type ViewOperationsClient } from "./viewOperations.js";
101 changes: 101 additions & 0 deletions packages/saved-views-react/src/viewOperations.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Bentley Systems, Incorporated. All rights reserved.
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import type { ViewStateProps } from "@itwin/core-common";
import {
DrawingViewState,
IModelApp,
SheetViewState,
SpatialViewState,
type ScreenViewport,
} from "@itwin/core-frontend";

import { applySavedView } from "./applySavedView.js";
import type { SavedViewData } from "./SavedView.js";

/** Minimal client interface required for {@link setView} when passing a saved view ID. */
export interface ViewOperationsClient {
getSavedViewDataById: (args: { savedViewId: string }) => Promise<SavedViewData>;

Check warning on line 19 in packages/saved-views-react/src/viewOperations.ts

View workflow job for this annotation

GitHub Actions / build

Expected a semicolon
/** Optional: list all saved views as `{ id, displayName }` pairs for name-based lookup. */
listSavedViews?: () => Promise<{ id: string; displayName: string }[]>;

Check warning on line 21 in packages/saved-views-react/src/viewOperations.ts

View workflow job for this annotation

GitHub Actions / build

Expected a semicolon
}
Comment on lines +18 to +22
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

listSavedViews returns { id, displayName }, but this package consistently uses savedViewId (see SavedView.savedViewId). This inconsistency forces consumers to special-case this API and increases the chance of mixing up different kinds of ids. Consider returning { savedViewId, displayName } (e.g., Pick<SavedView, "savedViewId" | "displayName">[]) and update the JSDoc to match.

Copilot uses AI. Check for mistakes.

let _client: ViewOperationsClient | undefined;

/**
* Initialize view operations with a client for ID-based {@link setView} calls.
* This is optional — {@link getActiveView} and {@link setView} with {@link ViewStateProps}
* work without initialization.
*/
export function initViewOperations(client: ViewOperationsClient): void {
_client = client;
}

function getActiveViewport(): ScreenViewport | undefined {
return IModelApp.viewManager.selectedView;
}

/**
* Returns the active viewport's current view state as {@link ViewStateProps}.
* Returns `undefined` if there is no active viewport.
*/
export function getActiveView(): ViewStateProps | undefined {
return getActiveViewport()?.view.toProps();
}

/**
* Lists all saved views accessible via the client supplied to {@link initViewOperations}.
* Returns an array of `{ id, displayName }` objects, or an empty array if no client is set.
*/
export async function listSavedViews(): Promise<{ id: string; displayName: string }[]> {

Check warning on line 51 in packages/saved-views-react/src/viewOperations.ts

View workflow job for this annotation

GitHub Actions / build

Expected a semicolon
Copy link
Copy Markdown

@adrianclinansmith adrianclinansmith Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe listSavedViews can be done from the backend with IModelDb.Views..

See queryViewDefinitionProps and getViewStateProps.

We only need to query the frontend when we want the current state of the viewport, but the saved views are stored in the iModel, which is on the backend.

The way it's done now: backend calls frontend -> frontend calls backend -> backend receives backend value

Copy link
Copy Markdown
Collaborator

@diegopinate diegopinate Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are 2 types of "views" that are used in applications:

  1. iModel Views: Contained in the iModel as you explain
  2. Saved Views: Created dynamically and stored in the saved views API

Using IModelDb.Views only would result in the first one.

return _client?.listSavedViews?.() ?? [];
}
Comment on lines +47 to +53
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JSDoc says listSavedViews returns an empty array only "if no client is set", but the implementation also returns [] when a client is set without listSavedViews implemented (_client?.listSavedViews?.() ?? []). Please either update the doc to reflect this behavior or change the implementation to fail loudly when listSavedViews is unavailable.

Copilot uses AI. Check for mistakes.

/**
* Sets the active viewport's view.
* - When passed a **saved view ID** (string), fetches the saved view data via the
* client supplied to {@link initViewOperations} and applies it.
* - When passed **{@link ViewStateProps}**, constructs a `ViewState` from the props
* and applies it directly — no client required.
* @returns `true` on success, `false` on any failure.
*/
export async function setView(viewIdOrProps: string | ViewStateProps): Promise<boolean> {
const vp = getActiveViewport();
if (!vp)
return false;

Comment on lines +63 to +67
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setView collapses multiple failure modes (no active viewport, missing client, client fetch failure, invalid props, view load failure, etc.) into a single false return and also swallows the underlying error. This makes it hard for callers to diagnose/configure failures. Consider either throwing (consistent with other exported APIs like applySavedView/createViewState) or returning a discriminated result that includes an error/reason code.

Copilot uses AI. Check for mistakes.
if (typeof viewIdOrProps === "string") {
if (!_client)
return false;

try {
const savedViewData = await _client.getSavedViewDataById({ savedViewId: viewIdOrProps });
await applySavedView(vp.iModel, vp, savedViewData);
return true;
} catch {
return false;
}
}

try {
const viewState = createViewStateFromViewStateProps(viewIdOrProps, vp);
await viewState.load();
vp.changeView(viewState);
return true;
} catch {
return false;
}
}

function createViewStateFromViewStateProps(
props: ViewStateProps,
vp: ScreenViewport,
) {
const className = props.viewDefinitionProps.classFullName.toLowerCase();
if (className.includes("drawing"))
return DrawingViewState.createFromProps(props, vp.iModel);
if (className.includes("sheet"))
return SheetViewState.createFromProps(props, vp.iModel);
Comment on lines +95 to +99
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

View type detection via classFullName.toLowerCase().includes("drawing"|"sheet") is brittle and can misclassify custom/derived view classes whose names contain those substrings. Prefer exact checks against DrawingViewState.classFullName / SheetViewState.classFullName / SpatialViewState.classFullName (and fall back to SpatialViewState if unknown) to make this deterministic.

Suggested change
const className = props.viewDefinitionProps.classFullName.toLowerCase();
if (className.includes("drawing"))
return DrawingViewState.createFromProps(props, vp.iModel);
if (className.includes("sheet"))
return SheetViewState.createFromProps(props, vp.iModel);
const className = props.viewDefinitionProps.classFullName;
if (className === DrawingViewState.classFullName)
return DrawingViewState.createFromProps(props, vp.iModel);
if (className === SheetViewState.classFullName)
return SheetViewState.createFromProps(props, vp.iModel);
if (className === SpatialViewState.classFullName)
return SpatialViewState.createFromProps(props, vp.iModel);

Copilot uses AI. Check for mistakes.
return SpatialViewState.createFromProps(props, vp.iModel);
}
Loading