iTwin / appui

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

SyncUiEventId.SelectionSetChanged no longer works during timeline animation #921

Closed Modeeeeeee closed 1 month ago

Modeeeeeee commented 1 month ago

Describe the bug

Basically our team (Synchro FieldApp) found out a bug that was marked as a regression that a selection pill no longer changes when animating the timeline. The selection pill is basically a simple counter that uses a hook which uses the SyncUiEventId.SelectionSetChanged. After upgrading some packages it no longer works when the timeline is being animated. Item link : Bug 1425663: iTwin.js 4.x - selection pill does not appear/update while timeline is playing Video : https://github.com/user-attachments/assets/2f923e33-f3a5-4090-96ea-bbe5caf09cf2

To Reproduce

  1. Open an iModel with a timeline
  2. Create a hook that uses SyncUiEventId.SelectionSetChanged for tracking count of selected items
  3. Play the timeline
  4. Observe selection set count changes when selecting items

Expected Behavior

The selection count should always be updated even when animating the timeline itself

Screenshots

Video : https://github.com/user-attachments/assets/2f923e33-f3a5-4090-96ea-bbe5caf09cf2

Desktop (please complete the applicable information)

OS - iOS/Android (mobile app) AppUi version - "@itwin/appui-abstract": "4.7.6",

Additional context

This issue occurred when upgrading from a lot of packages but specifically upgrading iModel.js from 2.19.x. At the time we were using "@bentley/ui-framework": "2.19.53" which is AppUi I suppose (I was directed to the AppUi team)

GerardasB commented 1 month ago

It seems like nothing changed in AppUI code-base itself. I.e. in https://github.com/iTwin/itwinjs-core/tree/release/2.19.x SyncUiEventId.SelectionSetChanged is dispatched only when selection set is changed https://github.com/iTwin/itwinjs-core/blob/1483045f6a7b06f6ee9d42270d5f99ec6d37cdcb/ui/framework/src/ui-framework/syncui/SyncUiEventDispatcher.ts#L324 This matches the current behavior: https://github.com/iTwin/appui/blob/a6c9cce62cf097c164fce53c2db80377ae0771fd/ui/appui-react/src/appui-react/syncui/SyncUiEventDispatcher.ts#L265

Are you certain that the selection set is modified?

Modeeeeeee commented 1 month ago

I suppose that the selection set has to be changed because after stopping the timeline animation the selection set does indeed update. Do you have any idea could it be an issue on a deeper level regarding selection set change listeners ?

Modeeeeeee commented 1 month ago

@GerardasB here I attached video of debugging that SyncUiEvent once the timeline is being animated and not, as you can see the selection set once the timeline is off is actually being modified, but once the timeline starts it does not. But once you stop the animation, the selection set count changes to the correct one

https://github.com/user-attachments/assets/8c52c7e9-e3d6-4554-bf45-e5dfa72eff83

GerardasB commented 1 month ago

What APIs are you using from AppUI packages? The Timeline component has no code to manage/disable the selection. If you are using AnalysisAnimationTimelineDataProvider all it does is set https://itwinjs.org/reference/core-frontend/views/viewport/analysisfraction/

Modeeeeeee commented 1 month ago

@GerardasB We are using ScheduleAnimationProvider. Everytime the timeline fraction changes we call the ScheduleAnimationProvider.onAnimationFractionChanged(fraction) to animate the timeline itself. Also a thing worth noting I think we are setting the viewport's continiousRendering to state of playing if that could have any significance finding what causes this issue.

GerardasB commented 1 month ago

As discussed, this seems like an application-specific issue, since AppUI doesn't manage the selection. Waiting for more details or a minimal repro case in case you find something else.

Modeeeeeee commented 1 month ago

@GerardasB the issue is in the library that provides the useSelectionCount hook on our end, but it broke after all deprecations. Decided to move away from AppUi's events for now and wrapping the selection set event by ourselves The issue can be closed