iTwin / appui

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

ToolSettings is not displayed #478

Closed Josh-Schifter closed 7 months ago

Josh-Schifter commented 1 year ago

ToolSettings is not displayed in come iTwin.js sandboxes. This appears to be a regression that occurred between appui 4.0 and 4.1.3.

The effect is inconsistent. Some users see ToolSettings and some do not.

We have reproduced the effect in the following sandbox, but it is not limited to this one. https://www.itwinjs.org/sandboxes/YashodipDeshmukh/Snappable-gltf-decoration

In this screenshot, the toolsettings are not displayed:

image

In this screenshot, the toolsettings are displayed:

image

• It appears that the cause may be a change that occurred between 4.0 and 4.1.3 versions of the AppUI packages. • The problem is NOT isolated to that one sandbox. • The problem is NOT isolated to our custom tools. In an affected session, we also do not see toolsettings for the Select tool. • We have reproduced on Chrome, but not all Chrome sessions are affected. • We have not been able to reproduce on Edge, but also we’ve done less testing there.

aruniverse commented 1 year ago

Should be resolved with 4.3.1

raplemie commented 11 months ago

Hi @Josh-Schifter, we havent released a 4.1.3 patch to AppUI, so I'm unsure of the request. (Is it a mix between core 4.1.3 release, or was it 4.3.1 typo ?) If the version was the core version, we are no longer in lock step with them, and I saw that sanboxes are now using 4.4.0, can you confirm if you still get this issue with 4.4.0, since from our side, this appears to have been fixed in 4.3.1 already.

Josh-Schifter commented 11 months ago

@raplemie, I can still reproduce this in multiple sandboxes in PROD. Use this link (copied from above): https://www.itwinjs.org/sandboxes/YashodipDeshmukh/Snappable-gltf-decoration or this one: https://www.itwinjs.org/sandboxes/iTwinPlatform/PrimitiveTool%20-%20Standard%20ToolSettings

In PROD, sandbox is currently using 4.1.3 of appui-abstract, and 4.4.0 of appui-react.

aruniverse commented 11 months ago

@raplemie, I can still reproduce this in multiple sandboxes in PROD. Use this link (copied from above): itwinjs.org/sandboxes/YashodipDeshmukh/Snappable-gltf-decoration or this one: itwinjs.org/sandboxes/iTwinPlatform/PrimitiveTool%20-%20Standard%20ToolSettings

In PROD, sandbox is currently using 4.1.3 of appui-abstract, and 4.4.0 of appui-react.

you prob need to clean your local storage settings

Josh-Schifter commented 11 months ago

@aruniverse, @raplemie, clearing the session cache seems to have fixed it. How will other users know to do that? Is there something we should be doing in the sandbox to clear it periodically?

aruniverse commented 11 months ago

@aruniverse, @raplemie, clearing the session cache seems to have fixed it. How will other users know to do that? Is there something we should be doing in the sandbox to clear it periodically?

not ideal, but id recommend running the RestoreFrontstageLayout tool anytime the sandbox is opened up https://github.com/iTwin/appui/blob/43958042424fe107ab35d5617eec195e3a6472e8/ui/appui-react/src/appui-react/tools/RestoreLayoutTool.ts#L25

raplemie commented 11 months ago

I think one of the issue is that the frontstage is always the same ID between sandboxes, yet change potentially greatly in configuration, so a bogus frontstage would override the stored value of every other sandboxes that do not define a specific name to their frontstages... And I'm not sure how "RestoreFrontstageLayout" tool would be hooked up automatically given that people have control over the Viewer. The most likely solution would be to clean the local storage related keys prior to starting the viewer, so we start fresh each time. Since this is a sandbox, I would assume that Closed/Open widgets and other configuration might be expected to be "clean slate".

raplemie commented 11 months ago

Another alternative if we want to keep the safe storage (Or simply ignore it) would be to provide a UiFramework.setUiStateStorage and potentially add the sandbox as part of the key when saving the settings, this way, the setting would not be messed up when moving from one sandbox to another, even if they use the same Frontstage Id, which we do not control by the nature of the sandbox.

claudiaareneee commented 9 months ago

Hey all! This still seems to be an issue.

The local storage entry for this is uifw-frontstageSettings.frontstageState[iTwinViewer.DefaultFrontstage]. Clearing just this variable resets it for me, but from a user perspective this is confusing.

It's especially confusing because I have my defaultUiConfig Viewer prop specified as so:

defaultUiConfig={
    {
        hideStatusBar: true,
        hideToolSettings: false,
    }
}

So, I would think the tool settings would show up, but they don't. Has any movement been made to fix this bug?

raplemie commented 9 months ago

Have there been any changes in the sandboxes ? I suggested 2 options and potential reasons for which the sandboxes where having this issue and did not get any followup.

GerardasB commented 7 months ago

Closing, since the original issue is supposedly fixed.