universalviewer icon indicating copy to clipboard operation
universalviewer copied to clipboard

Configuration doc WIP

Open Saira-A opened this issue 8 months ago • 24 comments

Work in progress. The following options need improved definitions:

  • [x] autoPlayOnSetTarget - DK
  • [x] confinedImageSize - DK
  • [x] imageSelectionBoxEnabled - DK
  • [x] limitToRange - GS [this setting gets passed to the AV component, but the AV component doesn't document what it does; still needs further investigation - footnote]
  • [x] metrics - GS
  • [x] mostSpecificRequiredStatement - SA
  • [x] multiSelectionMimeType - SA - unable to find manifest to test - footnote
  • [x] openTemplate - SA
  • [x] posterImageRatio - DK
  • [x] selectionEnabled - KS - unable to find manifest to test - footnote
  • [x] tokenStorage - DK
  • [x] visibilityRatio - DK
  • [x] zoomToBoundsEnabled - DK

and these options don't appear to work, or need more attention in the code (see #1449 for additional tracking of work on cleaning these up):

  • [x] currentViewDisabledPercentage - see comment below
  • [x] elideCount - see comment below
  • [x] forceImageMode - see comment below
  • [x] galleryThumbChunkedResizingEnabled - see comment below
  • [x] instructionsEnabled - see comment below
  • [x] optionsExplanatoryTextEnabled - see comment below
  • [x] pagingEnabled - see comment below
  • [x] pagingOptionEnabled - see comment below
  • [x] pessimisticAccessControl - see comment below
  • [x] saveUserSettings - see discussion in #1411
  • [x] seeAlsoEnabled - see discussion in #1411
  • [x] shareFrameEnabled - see comment below
  • [x] termsOfUseEnabled - see discussion in #1411
  • [x] theme - see comment below
  • [x] trimAttributionCount - see comment below
  • [x] zoomToSearchResultEnabled - see comments below

Saira-A avatar May 16 '25 11:05 Saira-A

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
universalviewer ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 26, 2025 3:48pm

vercel[bot] avatar May 16 '25 11:05 vercel[bot]

Thanks, @Saira-A -- I have turned your lists into checkboxes so that we can check things off as we improve them in the documentation (or otherwise resolve them).

demiankatz avatar May 16 '25 13:05 demiankatz

Regarding some the options reported as not working:

I believe that currentViewDisabledPercentage is intended to disable the "current view" option in the download dialog box at a certain zoom percentage, but the setting does not seem to be used. Was it lost in refactoring to React? We should either remove or reimplement it as appropriate.

I'm not sure what elideCount, galleryThumbChunkedResizingEnabled, instructionsEnabled, optionsExplanatoryTextEnabled, pessimisticAccessControl, shareFrameEnabled or trimAttributionCount were intended for, but they seem completely unused in the code (defined, but never read; in some cases with some commented-out, apparently useless logic). Unless @edsilv can think of a reason to keep them, we should probably open a PR to remove them so they don't cause future confusion.

The pagingEnabled option controls whether the viewer uses the single-page or two-page view, when the manifest supports 2-up display. I've found that changing the setting does impact the initial state of the viewer -- we use this at Villanova to force 1-up mode by default, since we have many manifests where 2-up is not ideal. However, there is definitely a lack of clarity in the interaction between this setting, the "Two page view" option in the settings dialog box, and the single/double page icons at the top of the OSD controls. I think some work may be needed to simplify/clarify/make things consistent.

My guess is that pagingOptionEnabled was intended to control whether or not the "Two page view" option appears in the settings dialog box, but it does not seem to be implemented at this time. Not sure whether we should remove it, or make it functional.

The forceImageMode setting also appears problematic; it seems like an override for pagingEnabled, but it only has an effect in the search footer panel. We have isPageModeEnabled methods defined in multiple modules following different rules -- it is very confusing and probably needs review and cleanup.

I strongly suspect that theme is a relic from past times when UV had a theme system (and we pulled in themes from separate Git modules). It's a bit hard to confirm this since the word "theme" is used so frequently in the code, so more careful investigation would be wise -- but I strongly suspect that we can remove this with no adverse effect. We should just make sure that eliminating it doesn't break i18n, since themes and locale used to be intertwined.

As far as I can tell, zoomToSearchResultEnabled is orphaned -- my suspicion is that it was superseded by zoomToBoundsEnabled. There is an unused isZoomToSearchResultEnabled() method that can probably be removed.

demiankatz avatar May 20 '25 12:05 demiankatz

Note: I've just alphabetized the list of "needs improvement" settings and consolidated the list of broken or problematic settings. Note that a checked status in the "improved definitions" list means "the setting works and the documentation has been improved" while a checked status in the "don't appear to work / needs more attention" list means "the setting has been investigated and commented on, but follow-up action is likely needed." I'm moving things from the top list to the bottom list when I discover them to be problematic.

demiankatz avatar May 21 '25 11:05 demiankatz

Regarding the limitToRange setting, there's a setting in the avcomponent that calls a _limitToRange function (AVCenterPanel.ts line 350). This function either returns the boolean for the limitToRange config if it's been set, if not returns true if the UV is not in "desktop metric" mode (and false otherwise).

isDesktopMetric returns true or false based on the string value of this.metric ("lg" or "xl" in regard to screen size/type) in BaseExtension.ts similar to isMobileMetric and isMetric.

These metric settings are set in the config as below: "metrics": [ { "type": "sm", "minWidth": 0 }, { "type": "md", "minWidth": 768 }, { "type": "lg", "minWidth": 1024 }, { "type": "xl", "minWidth": 1280 } ]

Then these are read in BaseExtension.ts and checked against the current screen dimensions. If the metric set in BaseExtension doesn't match the screen dimensions, the correct one is set and a METRIC_CHANGE IIIF event is published.

This is based on a relatively quick code read and I am more than open to folks' input in case I missed anything! I am also open to any recommendations on how to explain these settings in a simple, straightforward and terse way. :)

Geoffsc avatar May 21 '25 14:05 Geoffsc

Thanks for the investigation, @Geoffsc! I revised the metrics description in 620dc7bb46a7a956c20266f40a3088682230a5a3 based on your research; let me know if you feel further adjustment would be helpful. I'm not totally sure what to do with limitToRange since, as you say, that setting gets passed to the AV component, but I'm not familiar enough with the AV component to understand what it does once it goes there. Do we have any AV experts in the house?

demiankatz avatar May 21 '25 19:05 demiankatz

@demiankatz the revised metrics description looks good to me!

Geoffsc avatar May 21 '25 19:05 Geoffsc

@demiankatz I've just had a look at the theme option and can't find anywhere in the code that options.theme is referenced, so I think you're correct that it's a relic and can be removed.

LlGC-jop avatar Jun 12 '25 15:06 LlGC-jop

The zoomToSearchResultEnabled option and zoomToSearchResultEnabled() function are used by the OSD Center Panel and the Footer Panel, although zoomToBoundsEnabled is also used later in the zooming process.

So I think it's worth looking at exactly what happens when zoomToSearchResultEnabled and zoomToBoundsEnabled are true/false in various combos - without zoomToBoundsEnabled=true it will just pan to the result but I don't know what happens when zoomToSearchResultEnabled=false

I think the most sensible thing would be to either unify the options, or rename one of them to be more descriptive. Renaming would of course be a breaking change and have to wait until a minor release I think.

LlGC-jop avatar Jun 12 '25 15:06 LlGC-jop

pagingOptionEnabled exists only in the config files and isn't implemented anywhere in the code so I think we can scrap that. 'pagingEnabled' would seem to determine whether or not the user can switch to a 2-up view, but changing it doesn't do anything. This user can control this with the 2-up/1-up buttons if the manifest supports it. So the options seem to be to scrap it and let the user decide between 1/2-up where possible, or fix the code so that 2-up can be disabled even on manifests that support it. I suspect the second option might need some work to support various 'behaviour' options so not necessarily a quick fix.

jamesmisson avatar Jun 12 '25 15:06 jamesmisson

Put up patch PR to add extended description for selectionEnabled but when I did some digging I was unable to trigger the expected behavior - visible in this issue - in my own environment or in UV dev sandbox. This may require additional configuration that I did not employ. Would like to request further investigation to ensure this behavior is working as intended.

K8Sewell avatar Jun 16 '25 05:06 K8Sewell

Put up patch PR to add extended description for selectionEnabled but when I did some digging I was unable to trigger the expected behavior - visible in this issue - in my own environment or in UV dev sandbox. This may require additional configuration that I did not employ. Would like to request further investigation to ensure this behavior is working as intended.

@K8Sewell, it looks like the selectionEnabled setting has no effect if the manifest being used has no download service included -- see the selectionEnabled variable assignment in the OSD extension. Is it possible you were testing using a manifest with no download service? (I'm not personally sure where/how to find such a manifest, or if any currently exist -- I've never worked with bulk download myself).

demiankatz avatar Jun 16 '25 12:06 demiankatz

@K8Sewell @demiankatz

From what I've just been able to find I think the 'Service' part is something of a red herring. At least there's nothing in the code I can see that would use such a service, as opposed to something like the search service.

I don't know why Ed added it to the @iiif/vocabulary package in this commit - something to ask him at some point perhaps?

Hacking my local copy a bit I've managed to get a multi-select box up (though not in the gallery view), and all it ultimately seems to do is trigger the 'multiSelectionMade' event for the implementer to handle as they wish. It might be a good idea for us to at least provide a suggested implementation at some point.

So it's mostly as Ed outlined in #363, but it doesn't help that all the examples are utterly broken :) - something for an Admin sprint perhaps? The URLs in the vocabulary package that link to universalviewer.io are broken too...

LlGC-jop avatar Jun 16 '25 13:06 LlGC-jop

@LanieOkorodudu, I am also confused by saveUserSettings. The only impact I can definitely see it having has to do with the inclusion of the "remember settings" checkbox in the image adjustment control. That checkbox only appears when saveUserSettings is true. However, the code suggests that this setting should have broader impact, yet it doesn't seem to do what I would expect it to do, in terms of enabling/disabling use of browser storage more broadly. Browser storage seems to be used by default, even though the setting seems to be disabled by default. I think further investigation is needed.

@demiankatz it works as expected when set from the uv-iiif-config.json file rather than the individual config.json files, and whatever is set in that file is then set for all file types. There is a brief explanation of that on line 18 of the document but it might need further discussion - if options are instead just removed from the iiif-config file entirely then they do work in the regular config.json files instead which might be better for some, if not all of them

Saira-A avatar Jun 20 '25 15:06 Saira-A

@LanieOkorodudu, I am also confused by saveUserSettings. The only impact I can definitely see it having has to do with the inclusion of the "remember settings" checkbox in the image adjustment control. That checkbox only appears when saveUserSettings is true. However, the code suggests that this setting should have broader impact, yet it doesn't seem to do what I would expect it to do, in terms of enabling/disabling use of browser storage more broadly. Browser storage seems to be used by default, even though the setting seems to be disabled by default. I think further investigation is needed.

@demiankatz it works as expected when set from the uv-iiif-config.json file rather than the individual config.json files, and whatever is set in that file is then set for all file types. There is a brief explanation of that on line 18 of the document but it might need further discussion - if options are instead just removed from the iiif-config file entirely then they do work in the regular config.json files instead which might be better for some, if not all of them

If the setting is really just meant to show the “remember settings” checkbox in the image adjustment controls, then renaming it to something like enableSettingsCheckbox might make more sense. But if the original idea was for it to actually control whether user settings are saved or not, then it probably makes more sense to fix the implementation instead. That way, the name stays accurate, existing configs won’t break, and the behaviour will match expectations. I agree with Saira, would be great to hear what others think.

LanieOkorodudu avatar Jun 23 '25 10:06 LanieOkorodudu

It looks like @edsilv introduced uv-iiif-config.json in commit 47fb6970025f5eb929c20a5c5336db66e108fedd. This file only appears to be processed as part of the examples page and is not part of the application when it is deployed in other ways. I'm not sure why it was added, but perhaps it was just intended as a way to easily configure things for testing purposes, prior to the introduction of the config editor. Now that we have the config editor, perhaps we should consider removing it, unless it contains settings that are important to override for the purposes of the examples page. What do you think?

demiankatz avatar Jun 24 '25 11:06 demiankatz

For multiSelectionMimeType and selectionEnabled we’ve got definitions, but haven’t found suitable manifests to verify that they actually work. To prevent repeating the same statement we could add an asterisk next to those options with a note on the document like:
“* This option is documented based on its intended functionality, but a working example or environment to confirm its behaviour is yet to be identified.” That just leaves limitToRange from this list — do we need a similar statement for that?

Saira-A avatar Jun 24 '25 15:06 Saira-A

Thanks, @Saira-A -- I agree that using a footnote makes sense for this. Maybe the easiest thing to do is to treat limitToRange exactly the same as the other two.

We should also consider a separate footnote for the items in #1449 that are not yet resolved. Perhaps something like "There may be known problems and issues with this setting; it is in the process of being reviewed and its behavior may be revised in the future."

demiankatz avatar Jun 24 '25 15:06 demiankatz

@Saira-A, since we've discovered that uv-iiif-config.json is only used for examples, might it make sense to stop documenting individual settings used there? Assuming the setting definitions are the same and are documented elsewhere, maybe a description that just says "any settings included in this file will override equivalent settings from all modules" (or something like that). Just looking for an opportunity to simplify, if simplification would be appropriate.

demiankatz avatar Jun 25 '25 12:06 demiankatz

@Saira-A, since we've discovered that uv-iiif-config.json is only used for examples, might it make sense to stop documenting individual settings used there? Assuming the setting definitions are the same and are documented elsewhere, maybe a description that just says "any settings included in this file will override equivalent settings from all modules" (or something like that). Just looking for an opportunity to simplify, if simplification would be appropriate.

Yes that sounds like a good idea, will do

Saira-A avatar Jun 25 '25 12:06 Saira-A

The zoomToSearchResultEnabled option and zoomToSearchResultEnabled() function are used by the OSD Center Panel and the Footer Panel, although zoomToBoundsEnabled is also used later in the zooming process.

So I think it's worth looking at exactly what happens when zoomToSearchResultEnabled and zoomToBoundsEnabled are true/false in various combos - without zoomToBoundsEnabled=true it will just pan to the result but I don't know what happens when zoomToSearchResultEnabled=false

I think the most sensible thing would be to either unify the options, or rename one of them to be more descriptive. Renaming would of course be a breaking change and have to wait until a minor release I think.

I looked into it and found this:

zoomToSearchResultEnabled:

  • True - the viewer zooms in on a search result when selected.
  • False - still highlights the search result but doesn't zoom in at all.

zoomToBoundsEnabled:

  • Only has a visible effect if zoomToSearchResultEnabled is also true.
  • True - zooms in to the search result's bounds.
  • False - pans to the search result, but will only zoom into the current zoom level (unless you're zoomed in too far, then it just zooms into the bounds anyway)

So it may be worth keeping them separate but at the very least improving on their current descriptions @LlGC-jop @demiankatz

Saira-A avatar Jun 25 '25 16:06 Saira-A

Thanks, @Saira-A -- I'd agree that for now, just improving the descriptions is a good starting point. We could rename them at some point, but that's a lot more effort, and the focus right now is on documentation. Since you've proven that both settings have a purpose, we can start by clarifying how they interact. Let me know if you want any help drafting or reviewing anything.

demiankatz avatar Jun 25 '25 17:06 demiankatz

Thanks @demiankatz. From #1449 that just leaves these two:

currentViewDisabledPercentage - meant to hide the "download current view" option if the current view is the same as the whole image view. The code was lost when the download dialogue was refactored to use React so the config doesn’t work anymore - I think the easiest thing to do would be to just remove it from the config files as well and then create a new issue to add it back in at a later date

forceImageMode - only appears to be used in the search footer panel, where it changes the page count to image count. Could just add that to the documentation but keep the footnote warning that it's still being reviewed.

Does this sound ok to you?

Saira-A avatar Jun 26 '25 11:06 Saira-A

@Saira-A, that sounds like a good approach to finish this up; I agree -- let's get rid of currentViewDisabledPercentage and document forceImageMode as-is for now. As you say, we can open an issue to revisit currentViewDisabledPercentage in future.

demiankatz avatar Jun 26 '25 11:06 demiankatz