2D Navigation Aid appears when it shouldn't

jason-crow commented 1 month ago

Describe the bug

previously, when a ViewportContentControl overrides the navigationAidControl property to return an empty string, this would prevent any navigation aid from showing. This was used to suppress the default 2d navigation aid from showing. Now the 2d navigation aid appears despite the override.
A new issue is showing where when a ViewportControl is opened in a widget the navigation aid of the viewport in the view is replaced with the one applicable to that new ViewportControl. This means if a 2d viewport is opened in viewport control inside a widget, the main view, which could still be 3d, will now show the 2d navigation aid
both these issues are breaking changes introduced in 4.16.0

To Reproduce

Default 2d Navigation Aid is no longer suppressed by navigationAidControl override

make the main frontstage use a viewport control with logic like

override get navigationAidControl() {
const viewClassName = this.viewport?.view.classFullName;
if (
  !this.options?.featureOptions?.enable2dNavigationAidControl &&
  (viewClassName === DrawingViewState.classFullName ||
    viewClassName === SheetViewState.classFullName)
) {
  return ""; // disable navigation aid for 2d views
}

return super.navigationAidControl;
}

open a 2d view
notice the default 2d navigation cube appears

2d Navigation Aid shows in main view when opening a 2d view in a viewport control even when the main view remains 3d
have a frontstage with a widget using a viewport control
open a 3d view
open a 2d view in the widget with a viewport control
notice the main view, which is still showing a 3d view, now shows the 2d navigation aid

Expected Behavior

Expected behavior is for the default 2d navigation cube to continue to be hidden when overriding the navigationAidControl to return an empty string and also for the navigation aid of the main view to be unaffected by the viewport control that a widget maybe using

Screenshots

No response

Desktop (please complete the applicable information)

No response

Additional context

reproducible in 4.16.0

jason-crow commented 1 month ago

@raplemie @jjbeckman13

raplemie commented 1 month ago

@jason-crow The second part of the expectation, that the navigation aid main view being unaffected by a widget viewport would be a new behavior, because all other tools are dependent/visible based on the active viewport, so it is not new with 4.16.

The first expectation used to work in 4.14.4 though (didnt test with 4.15)

@GerardasB The deprecation of the ViewportContentControl may be related to this, but that deprecation is problematic because the suggested new way, overriding the ViewToolWidgetComposer props, requires a change in architecture of our library, because when we provide a widget, we dont provide these props to say what navigation aid should be active...

jason-crow commented 1 month ago

@raplemie you're right! I didn't realize this was already an issue, but the only new part seems to be the default 2d navigation cube , which made me notice this issue

This is 4.14.1 which demonstrates the issue only the navigation cube is hidden unlike in 4.16 Recording 2024-08-13 at 13 55 05

jason-crow commented 1 month ago

@raplemie just to point out its not a unique problem for the sheet viewer, because the 2d frontstage also has the same issue, which is why i mentioned the navigationAidControl override because that was what was previously preventing the default from showing

this shows both the 2d frontstage and sheet viewer demonstrating the issue Recording 2024-08-13 at 13 59 51

GerardasB commented 1 month ago

https://github.com/iTwin/appui/pull/970 should address the regression where returning an empty string for a navigation aid control would not be respected and a default navigation aid would still be rendered.

iTwin / appui