search

imodeljs / itwin-viewer

Monorepo that contains the iTwin Viewer npm package and its related packages
MIT License
15 stars 13 forks source link

add the ability to provide UI providers to the viewer #74

Closed kckst8 closed 3 years ago

kckst8 commented 3 years ago

lgtm, thanks for getting this done so quickly.

my only concern is if someone needs an active iModelConnection when they are trying to initialize a startup extension or adding a new UiItemsProvider. if i'm not mistaken, the active iModelConn gets set after viewer has been initialized with all extensions and UiItemsProviders loaded.

Need a way in core to listen to changes on iModelConnection changed or something like that

We could probably move the registration to iModelLoader after the connection. My concern there is if there is a case where consumers may want them registered earlier (for custom frontstages, etc) and also the added complexity around duplicate registrations since the iModelLoader will typically re-render...I wonder how often it is the case of needing the iModelConnection for the UI and if that is an edge case where they would just use the onIModelConnected callback and register themselves.....thoughts?

aruniverse commented 3 years ago

lgtm, thanks for getting this done so quickly. my only concern is if someone needs an active iModelConnection when they are trying to initialize a startup extension or adding a new UiItemsProvider. if i'm not mistaken, the active iModelConn gets set after viewer has been initialized with all extensions and UiItemsProviders loaded. Need a way in core to listen to changes on iModelConnection changed or something like that

We could probably move the registration to iModelLoader after the connection. My concern there is if there is a case where consumers may want them registered earlier (for custom frontstages, etc) and also the added complexity around duplicate registrations since the iModelLoader will typically re-render...I wonder how often it is the case of needing the iModelConnection for the UI and if that is an edge case where they would just use the onIModelConnected callback and register themselves.....thoughts?

Yea i dont really love the idea of deferring registering these providers after imodel connected, and if we were to do that, i think we'd want to do the same to extensions. I guess in theory everything at the ui layer should be imodel agnostic so it makes sense with the way it is.

With some of the work im doing atm with SharedMeasurements, theres a bindToIModel call i need to make with an imodelconn before registering my provider. I dont love this and i think it might just be better for me to refactor this, but for the time being i will continue registering my provider and doing my initialization logic in the onImodelConnected callback