iTwin / appui

Monorepo for iTwin.js AppUi
MIT License
8 stars 2 forks source link

IModelViewportControl does not handle active iModelConnection changes #449

Open ImVeryLost opened 11 months ago

ImVeryLost commented 11 months ago

Describe the bug

To Reproduce Steps to reproduce the behavior:

  1. Create an BriefcaseConnection
  2. Create and set a new frontstage:
    
    new StandardFrontstageProvider({
    id: frontstageId,
    usage: StageUsage.General,
    contentGroupProps: new DefaultContentGroupProvider(),
    });

class DefaultContentGroupProvider extends ContentGroupProvider { public override async contentGroup(): Promise { const iModelConnection = UiFramework.getIModelConnection(); await registerToolProviders(frontstageId); assert(!!iModelConnection, "No iModelConnection"); const view = await getViewState(iModelConnection); UiFramework.setDefaultViewState(view);
return new ContentGroup({ id: "test-content-group", layout: StandardContentLayouts.singleView, contents: [ { id: "no-split", classId: IModelViewportControl, applicationData: { viewState: view, iModelConnection, defaultContentTools: { vertical: { selectElementGroupPriority: 100, measureGroupPriority: 200, selectionGroupPriority: 300, }, horizontal: { clearSelectionGroupPriority: 100, overridesGroupPriority: 200, }, }, }, }, ], }); } }


3. Without changing the active frontstage, iModelConnection is changed:

activeConnection.close(); .... ... const connection: BriefcaseConnection = await BriefcaseConnection.openFile(...); UiFramework.setIModelConnection(connection)


4. The viewport becomes blank.

**Expected behavior**
Viewport and viewState should be recreated

**Screenshots**
If applicable, add screenshots to help explain your problem.

**Desktop (please complete the applicable information):**

- OS: [windows]
- Browser [electron]
- iTwin.js AppUI Version [4.2.0-dev.1]
- iTwin.js Version [4.1.0-dev.77]

**Additional context**
Add any other context about the problem here.

<!--

If you can also contribute a fix, we'd absolutely appreciate it!

Check out the contributor guide to get started:

https://github.com/iTwin/appui/blob/master/CONTRIBUTING.md

Just let us know you're working on it and we'd be happy to provide advice and feedback.

-->
NancyMcCallB commented 10 months ago

You are correct that none of the content controls react to an IModelConnection change. Up to this point, the workflow has been to recreate the Frontstage under these circumstances. I'll look at a fix starting today.

NancyMcCallB commented 10 months ago

Hi, @ImVeryLost,

I've been trying to find a generic way to autmatically update the IModelViewport when the IModelConnection changes, but there simply is not enough information. We do not know the view you wish to display, for example, or how many views you have displayed in your content area. You set all that up in your content provider, but if the frontstage does not get recreated, there's no reason to call the content provider.

I'm concerned about changing the default behavior of the Frontstage or the setIModelConnection() method. I could add a method to IModelViewportControl that refreshes with a new IModelConnection and ViewState. Would that work for you?

ImVeryLost commented 10 months ago

@NancyMcCallB

That sounds similar to what I'm currently doing to get it to work:

export class WIPViewportControl extends ViewportContentControl {
  private _viewState?: ViewState;

  constructor(info: ConfigurableCreateInfo, options: IModelViewportControlOptions) {
    super(info, options);

    const iModelConnection = UiFramework.getIModelConnection();
    const viewState = UiFramework.getDefaultViewState();
    if (iModelConnection && viewState)
      this.reactNode = this.getImodelViewportReactElement(iModelConnection, viewState);

    IModelConnection.onOpen.addListener(async (connection) => {
      // viewport should react to connections changes.
      this._viewState = await getViewState(connection);
      if (this.viewport)
        this.viewport.applyViewState(this._viewState); 
      this.reactNode = this.getImodelViewportReactElement(connection, this._viewState);
    });
  }

  /** @inheritdoc */
  protected getImodelViewportReactElement(
    iModelConnection: IModelConnection,
    viewState: ViewStateProp,
  ): React.ReactNode {
    return (
      <UnifiedSelectionViewport
        viewState={viewState}
        imodel={iModelConnection}
        controlId={this.controlId}
        viewportRef={(viewport: ScreenViewport) => {
          this.viewport = viewport;
          if (!UiFramework.frontstages.isLoading)
            UiFramework.frontstages.activeFrontstageDef?.setActiveViewFromViewport(viewport);
        }}
      />
    );
  }
}

Simplifying the above would be great though, so a method like that could work. My only concern is that I do not currently initialize the ViewportControl, I just pass the whole thing to the Content provider:

return new ContentGroup({
      id: "content-group",
      layout: StandardContentLayouts.singleView,
      contents: [
        {
          id: "no-split",
          classId: WIPViewportControl,
          ....

Will this new method be static, or will I need to somehow find my "active viewport control"?