REVIEW: Helper objects and methods for canvas resources

[nein09]

For https://github.com/sul-dlss/iiifManifestLayouts/issues/40

[iangilman]

@nein09 Looks like we've been using two space indents for this project (which, incidentally, is what I prefer). Please change canvasObject.js to match. And I suppose canvasLayout.js while you're at it.

[iangilman]

FWIW, I'm all for using Underscore...I just want to make sure @aeschylus is cool with it.

[nein09]

Don't worry- I took underscore out that time because I wasn't using it anymore - it was nice when I was just merging keys, but I wanted to be more explicit about building up that object.

[mejackreed]

Prefer not to include another library if not necessary.

[nein09]

That makes sense- I initially included it because it's in package.json anyway, but I see now that it's part of the dependency chain here, not explicitly chosen as something that we want to include.

If I really feel like I want it for something, we can talk about this some more, but for now, I'm fine with leaving it out.

[iangilman]

@nein09 I've added a sublime project to this branch; if you use it, it can help you keep your tab settings in line. Also note that it's excluding the node_modules from the file list; this makes multi-file searching more pleasant. We might consider also excluding build results.

[nein09]

Okay, I think I've got this in a good place now. I added some properties and methods to canvasObject to make it more of a full-fledged object - @aeschylus , is there anything else you'd like to see in there in order to consider https://github.com/sul-dlss/iiifManifestLayouts/issues/40 resolved?

Currently, the viewer loads thumbnails initially, and doesn't load the actual tileSource until you click on the image. We should talk about when else to load the actual tileSource - knowing that you're in detail mode on a selected canvas seems like a good time. When that happens, I think I'd want to change to a more fine-grained event system, as we discussed last week.

Please note that I'm using the same dummy thumbnail for each image right now- we should talk about how to move forward with the real ones (does IIIF have a thumbnail attribute in the manifest? If so, what do we do when it's missing?).

[iangilman]

We'll also want to load the actual tileSource if you're in filmstrip mode and you pan over to it.

One approach would be to watch zoom and pan events and check to see if a tiledImage is featured enough to swap. That way we don't need to pay attention to specific scenarios that might feature a canvas.

[nein09]

Thanks! I don't mind where DS_Store gets ignored; it's easy enough for me to make that global.

[nein09]

Passing the position to the constructor means changing where/when the canvasObjects get constructed - right now, they're built before a layout is calculated. I could make the objects be constructed on the fly, in enterImages, though.

I also added some helpers to canvasObject so that we won't have to reference mainImageObj directly (like we do now in translateTileSources) - I think it'd be nicer to make mainImageObj a private member of CanvasObject. What do you all think?

[iangilman]

I like the latest commit...you can pass the x and y into the constructor or call setPosition later...that gives the most flexibility as to when the canvasObjects get created vs. when their location is determined.

And yes, I'm in favor of mainImageObj being private to CanvasObject; that should drive some good encapsulation.

[iangilman]

Beyond that minor comment, it's looking great to me! The other thing that still needs doing is getting the actual thumbnails in there, which hopefully @aeschylus can fill us in on.

[iangilman]

Changes look good to me.

[iangilman]

I've made a couple of commits...just little browser compatibility things.

[iangilman]

Changes look good to me. I think this is ready to merge. Can you please confirm, @aeschylus?