iTwin / appui

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

Changing native tool settings widget state does not work #515

Closed GintV closed 9 months ago

GintV commented 11 months ago

Describe the bug Let's say I define toolSettings and any other widget like so in frontstage config

toolSettings: {
  id: "NativeToolSettingsWidget",
  isToolSettings: true,
},
leftPanel: {
  sections: {
    start: [{
      id: "NormalWidget", 
      ...
    }]  
  }
}

While in frontstage "A", I hide both widgets:

["NativeToolSettingsWidget", "NormalWidget"].map(id => 
  UiFramework.frontstages.activeFrontstageDef?.findWidgetDef(id)?.setWidgetState(WidgetState.Hidden)
)

Then I open them again just before exiting frontstage "A":

["NativeToolSettingsWidget", "NormalWidget"].map(id => 
  UiFramework.frontstages.activeFrontstageDef?.findWidgetDef(id)?.setWidgetState(WidgetState.Open)
)
await UiFramework.frontstages.setActiveFrontstage(undefined);

Expected behavior Once I return to the frontstage "A" I see both widgets opened. I would expect tool settings widget to be treated as other widgets.

Actual behavior Only NormalWidget is open.

Desktop (please complete the applicable information):

Additional context

Related to #372

raplemie commented 11 months ago

Thanks, we'll look into it !

raplemie commented 10 months ago

Hi @GintV, I could not reproduce this issue with 4.6.3, can you validate if this is still a problem ?

Below is an example where the Step 1 button is using a copy of your code to hide both widgets, and Step 2 is using a copy of your code to Open and switch to the "Custom Frontstage" (as undefined is not an option)

https://github.com/iTwin/appui/assets/1904889/0e58efc4-9890-4d45-91bc-bb26ada26f26

You mention that you change this "just before exiting", as this is example code, maybe the production code is actually doing this in an event, could it be the case ? If so, can you tell me which event you are using so I have the same test case?

(I'm pretty sure I already wrote this answer but I cant find it... So, if you see this for the second time, let me know where I wrote this first! :) )

GintV commented 9 months ago

@raplemie I still can reproduce this with 4.6.3 . In your step 2, "switching" to undefined is the key, it is not reproducable if you switch between frontstages.

I generalised it as switching to undefined, what we do exactly there is we switch route to the one that doesn't have ConfigurableUiContent available (alongside setting active frontstage to undefined).

raplemie commented 9 months ago

UiFramework.setActiveFrontstage do not allow undefined as a value, and will do nothing if it doesnt find a fronstage with the id provided. However for some reason UiFramework.setActiveFronstageDef to undefined is valid, so I'll test with that.

What do you mean by having "no ConfigurableUiContent available" ? Is it simply no longer displaying AppUI at all?

GintV commented 9 months ago

@raplemie Yes, we're calling UiFramework.frontstages.setActiveFrontstageDef(undefined) . And yes, we're no longer displaying AppUI. Maybe there is some starter appui code sandbox (with frontstage and some widgets defined already), I could try to reproduce it in there if needed.

raplemie commented 9 months ago

Ok, the issue is not with tool settings but with the way we save the information, when changing any settings, there is a debounce of 1 second before we write to the local storage, if by this time you have moved to a different frontstage, it will not save. (When remaning in AppUI, and returning to the previous fronstage will actually reused the "live" data, and save it to local storage so it does appear to work correctly)

I'll look if I could bail out of the debounce when the stage is closing, so we keep the information all the time.

raplemie commented 9 months ago

Hi @GintV, a fix for this have been released today in AppUI 4.6.4. Let us know if this fixes your issue correctly!

GintV commented 9 months ago

@raplemie I still can reproduce this issue. I debugged it and here is what I found:

  1. saveSetting successfully sets handler to save latest state in store.subscribe;
  2. saveSetting.cancel() gets called which sets timeout to undefined;
  3. pendingState.current() gets called, but since timeout is undefined, handler doesn't execute.

image

I manually hacked timeout to be defined in step 3 and everything worked.

raplemie commented 9 months ago

Hi @GintV, an additional fix was released today in 4.7.0. Let us know if this fixes your issue!

GintV commented 8 months ago

I can confirm issue is now solved, thanks!