candela
candela copied to clipboard
Kitware
ResLab makes server roundtrip when it probably shouldn't
To reproduce, start a new project, click on "add data set", wait for the list of data sets to appear, then resize the window. Note that the window "flashes" as it updates the list of data sets, and new HTTP requests appear in the Girder log.
It seems like the view and model that maintain the list of current data sets shouldn't be listening to the window resize event in order to refresh themselves.
There isn't actually a model that contains the list of "current" datasets. To get the freshest stuff possible, things are rendered directly from the results of the call to the API endpoint—no state is stored locally.
This isn't the proper way to do things in terms of Backbone—even for this kind of approach, ideally, you'd create a one-off collection of models and call fetch(). But Girder breaks a lot of Backbone's fetch/sync behavior, especially where metadata is concerned (and we only have patches for item-specific stuff in MetadataItem.js... we haven't touched patching collections yet), so it's easier to just render from the API call directly. Alternative ideas to this approach are welcome.
When I get a chance, I agree we should pull the API call out of the render function (only call it the first time the panel is shown), and rewrite the D3 code more elegantly to handle resizing without wiping the DOM—this should get rid of the flash without having to patch Backbone.
Does a resize destroy the entire DOM?
I think the fix for this issue is just to cache the list of data sets in the data set list view, though it probably isn't worth the effort if it's not causing us problems right now, and the truer fix is to wait for Girder to provide better Backbone contract behavior.
In the meantime, I can take a look and see if I can cook up a quick fix that doesn't disrupt things too much.
No, just that view—it was a quick way to get something working. To do it properly, a careful review of enter vs update is probably in order. In a sense, the list is "cached" in the DOM the first time it's rendered, and smarter D3 code will respect that (just rearrange existing elements based on the new size).
Oh I see. Ok, well let's just leave this issue in place for now and we can revisit if it becomes more urgent or else once bigger things clear from the queue. Thanks for the explanations.