Skip to content

Configuration doc WIP#1419

Merged
demiankatz merged 31 commits intodevfrom
config-wip
Jun 30, 2025
Merged

Configuration doc WIP#1419
demiankatz merged 31 commits intodevfrom
config-wip

Conversation

@Saira-A
Copy link
Copy Markdown
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 - footnote]
  • metrics - GS
  • mostSpecificRequiredStatement - SA
  • multiSelectionMimeType - SA - unable to find manifest to test - footnote
  • openTemplate - SA
  • posterImageRatio - DK
  • selectionEnabled - KS - unable to find manifest to test - footnote
  • 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

@vercel
Copy link
Copy Markdown

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 26, 2025 3:48pm

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

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

@Saira-A
Copy link
Copy Markdown
Contributor Author

Saira-A commented Jun 25, 2025

@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

Remove uv-iiif-config.json duplicate options
Add footnote [^2]
saveUserSettings
@Saira-A
Copy link
Copy Markdown
Contributor Author

Saira-A commented Jun 25, 2025

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

@demiankatz
Copy link
Copy Markdown
Contributor

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.

@Saira-A
Copy link
Copy Markdown
Contributor Author

Saira-A commented Jun 26, 2025

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?

zoomToBoundsEnabled
@demiankatz
Copy link
Copy Markdown
Contributor

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

forceImageMode
Copy link
Copy Markdown
Contributor

@demiankatz demiankatz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @Saira-A -- nearly done, but I think this latest one could use some clarification (and sorry that my poor memory prevents me from recalling WHICH clarification is the right one).

I also think the last finishing touch before we take this out of draft mode and merge it is to decide on where the file should live -- maybe we can discuss that during the retrospective.

Comment thread CONFIG.md Outdated
remove currentViewDisabledPercentage
update forceImageMode
Copy link
Copy Markdown
Contributor

@demiankatz demiankatz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @Saira-A!

@demiankatz demiankatz merged commit 2577eaa into dev Jun 30, 2025
6 checks passed
@demiankatz demiankatz deleted the config-wip branch June 30, 2025 14:45
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.

8 participants