Skip to content

Configuration doc WIP #1419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 16 commits into
base: dev
Choose a base branch
from
Draft

Configuration doc WIP #1419

wants to merge 16 commits into from

Conversation

Saira-A
Copy link
Contributor

@Saira-A Saira-A commented May 16, 2025

Work in progress. The following options need improved definitions:

  • autoPlayOnSetTarget - DK
  • confinedImageSize - DK
  • imageSelectionBoxEnabled - DK
  • limitToRange - GS [this setting gets passed to the AV component, but the AV component doesn't document what it does; still needs further investigation]
  • metrics - GS
  • mostSpecificRequiredStatement - SA
  • multiSelectionMimeType - SA - unable to find manifest to test
  • openTemplate - SA
  • posterImageRatio - DK
  • selectionEnabled - KS - unable to find manifest to test
  • tokenStorage - DK
  • visibilityRatio - DK
  • 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):

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

Copy link

vercel bot commented May 16, 2025

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 20, 2025 0:36am

@demiankatz
Copy link
Contributor

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
Copy link
Contributor

demiankatz commented May 20, 2025

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
Copy link
Contributor

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.

@Geoffsc
Copy link
Contributor

Geoffsc commented May 21, 2025

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. :)

@demiankatz
Copy link
Contributor

Thanks for the investigation, @Geoffsc! I revised the metrics description in 620dc7b 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?

@Geoffsc
Copy link
Contributor

Geoffsc commented May 21, 2025

@demiankatz the revised metrics description looks good to me!

@LlGC-jop
Copy link
Contributor

@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
Copy link
Contributor

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.

@jamesmisson
Copy link
Contributor

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.

openTemplate / openEnabled definitions
@K8Sewell
Copy link

K8Sewell commented Jun 16, 2025

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.

@demiankatz
Copy link
Contributor

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).

Remove items from  #1451
@LlGC-jop
Copy link
Contributor

@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...

mostSpecificRequiredStatement
termsofUseEnabled
remove trimAttributionCount
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants