Allow for annotating image-sequences when annotations not already present #18
Conversation
|
This will mega-conflict with #17 @BryonLewis was first, so the git gods command ye to rebase (after he merges). :( |
jjnesbitt
force-pushed
the
fix_annotations
branch
from
062efe9
to
723e493
Compare
|
This should be good regarding conflicts with #17. Since I've rebased off that branch, this PR should wait until that one is merged. |
| }); | ||
|
|
||
| detections = data | ||
| ? data.map(detection => Object.freeze(detection)) |
There was a problem hiding this comment.
I know it was never commented before, but using object.freeze to protect objects from reactivity like this really deserves some explanation. Some developer will come along later and wonder what was going on.
There was a problem hiding this comment.
I'm not sure why it's being done either, it was there before I came onto the project. I think it's deserving of it's own PR though.
client/src/views/Viewer.vue
Outdated
| @@ -422,6 +422,11 @@ export default { | |||
| /* END TODO */ | |||
| async loadDataset(datasetId) { | |||
| var { data: dataset } = await this.girderRest.get(`folder/${datasetId}`); | |||
|
|
|||
| if (!dataset) { | |||
| throw "Could not fetch dataset!"; | |||
There was a problem hiding this comment.
I found an old stackoverflow thread that says this behaves unexpectedly in IE and safari, but that was in 2012. My gut says throw new Error() should always be used, but maybe you know something I don't.
This addresses the major concern raised by @mattdawkins. The diff is fairly large, as there needed to be several changes to get this to work, including fixing usability.