Add menu for canvas choice#1748
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@jamesmisson is attempting to deploy a commit to the Universal Viewer Team on Vercel. A member of the Team first needs to authorize it. |
demiankatz
left a comment
There was a problem hiding this comment.
Thanks, @jamesmisson, this looks like it's off to a good start. See below for some questions and suggestions.
Also, a more general question: I see that you have logic to make this work in 2-up mode, but what happens if each canvas in 2-up view has a different set of choices? I'm not sure how the UI would work in that situation... but maybe that will be more clear when you provide the more complex sample manifest you alluded to in your comments.
|
Hi @demiankatz , thanks very much for taking a look. I think on reflection I should rethink the 2-up view. As you say, they might have different choices; my current approach assumes there's at least some overlap in the choices and tries to merge them into one list. A more generalizable approach would be to have two sections in the choice menu, one for each canvas, if 2-up view is enabled. I'll work on that this afternoon. |
demiankatz
left a comment
There was a problem hiding this comment.
Great progress, @jamesmisson, I think we're nearly done -- but see below for a few remaining questions and suggestions.
|
Thank you very much @demiankatz , I've now addressed those comments |
demiankatz
left a comment
There was a problem hiding this comment.
Thanks for the progress, @jamesmisson -- see below for a couple of followup questions.
|
Thanks @demiankatz , I've made those changes |
demiankatz
left a comment
There was a problem hiding this comment.
Thanks, @jamesmisson, nearly done, but see below for a couple more i18n-related tweaks.
|
Thanks @demiankatz , that's done! |
demiankatz
left a comment
There was a problem hiding this comment.
Thanks, @jamesmisson! I think I may have spotted a typo in the latest changes.
demiankatz
left a comment
There was a problem hiding this comment.
Thanks, @jamesmisson, I think this is in pretty good shape now!
It would be great to add some test coverage, since some aspects of this functionality are potentially fragile (e.g. the way the button gets inserted, should we change other buttons, and the OSD integration, should any internal APIs change in a future version). Having good tests will likely prevent future problems. But that's not a prerequisite for merging this -- just something that @LanieOkorodudu should consider adding to her TODO checklist.
This adds support for manifests using 'choice' on the canvas as part of the BL's multispectral work, adding a dropdown menu to the image control buttons to switch between layers. I'll leave it in draft as there are a couple of outstanding things.
Most of the logic occurs in the openMedia function in OpenSeadragonCenterPanel, which now checks for choices and branches off into a slightly different way of handling image display.
This currently works for 1-up or 2-up views,
provided the canvases all have at least one choice.I don't think this will work in a scenario where the first of the canvas pair does not use choice and the second one does (because if a choice is detected on either, openMedia branches off into rendering choices only). I'll come back to this.DoneIt should work if canvases have a different number of choices and at least 1 choice (though I still need to create a manifest to test this).
I've added the cookbook choice recipe to the example manifests. We're currently working on a more substantial manifest to test this.
I've used the existing dialogue component for the dropdown choice menu, but have developed it slightly to be positionable above or below its anchor.
I had some problems rebasing this onto v4.4.0 so the commits might look a bit weird. The work in progress can be seen on my original branch: https://github.com/jamesmisson/universalviewer/tree/choice
To do:
Add choice menu to mobile footerout of scopeMake a config option for preserving choice selection over canvas changesthis gets complicated in 2-up mode with different numbers of choice per canvas, out of scopeI think ultimately the image adjustment dialogue (brightness etc.) should be streamlined and restyled to resemble this one.