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

@LlGC-mag, Just a quick note, while testing with a custom config, I was a bit confused by the 'Clear saved viewer state before applying' option. I expected it to remove what I had just pasted, similar to how 'Clear annotation' works. But it seems to keep the pasted config regardless. I know this is still a WIP, and my feedback might be a bit out of scope for this PR, but I just thought I’d mention it in case it helps with refining the config workflow.

@LanieOkorodudu, the "Clear saved viewer state" erases the browser's storage to reset the viewer state to default (e.g. forgetting any panel open/closed states that were stored as the result of user action). This is important because if you change default panel settings but the viewer remembers user activities, it appears as if the configuration had no effect, because the browser storage overrides the configurations. Maybe we need a better label for this checkbox to make its behavior more clear. Do you have any suggestions? Maybe something like "Reset browser storage to default state when applying configuration"?

@LanieOkorodudu Maybe the wording needs changing? The function is to remove any user settings stored by the browser which override the config which is pasted in or loaded by default. That work was in #1407 but it is required for the accordion menu underneath the main options.
At the moment, this draft PR is mainly to show what is loaded in as a config when the viewer is rendered so we can get an idea of the available options. I'm planning to expand it into something with more functionality but it's very basic at the moment - more a proof of concept

@LlGC-mag, since #1407 has now been merged, you should be able to merge the dev branch back into this branch to reduce the diffs showing in this PR.

I took a closer look at this, and it's off to an excellent start! A few comments:

1. It seems like applying a custom config creates multiple accordions on the page. That probably won't be a problem in the long term if we create a target element on the page to inject the accordion into, rather than appending it to the end of the document. Maybe we should discuss where that would be most useful/appropriate -- above or below the config box, perhaps?
2. It might be helpful to alphabetically sort the options at each level for findability/readability.
3. Is the planned next step to make things editable in the tree? Seems like it might not be a huge step from here to make the individual items clickable so that they turn into inputs, and editing the inputs adjusts the JSON in the custom config box. There would be a little complexity in merging the existing config with new configs, and in type-sniffing to format the content from the input box correctly in the JSON, but I don't think those would be insurmountable problems.
4. The code needs some style tweaking (inconsistent indentation, etc.). I know it's in a draft condition, so you're probably already aware of this, but we'll want to clean it up a little when it's closer to being finalized!
5. Can some of the commented out code be removed at this point to clean things up?

@demiankatz I've just pushed some changes and I hadn't seen your message until now. Most of the things you mention are things I've added:

1. Dropping the accordion in at the end of the page was temporary - it should sit better in the page now.
2. Makes sense to do this - not sure on the complexity of it with it being nested objects but I can have a look
3. Yes, that was the intention. Boolean values should now be true/false select options with everything else in a text input. Still some work to do on using it to update the config
4. The code is awful :D but at the moment I'm just getting everything working and capturing it before I fiddle too much and break it. Once there's something working I'll go through and tidy the code and the inline styles etc.
5. I've removed a bit of commented code - just leaving in the bits I may need in the future for now

I imagine that I'll get this branch in a useable state, get it tested, then create a new branch and tidy things up before making a new PR

Thanks, @LlGC-mag -- all sounds good! I don't think there's a need to create a separate PR, though; I see no disadvantage to just getting this one in shape and then squash-and-merging it when it's ready. :-)

@LanieOkorodudu, the "Clear saved viewer state" erases the browser's storage to reset the viewer state to default (e.g. forgetting any panel open/closed states that were stored as the result of user action). This is important because if you change default panel settings but the viewer remembers user activities, it appears as if the configuration had no effect, because the browser storage overrides the configurations. Maybe we need a better label for this checkbox to make its behavior more clear. Do you have any suggestions? Maybe something like "Reset browser storage to default state when applying configuration"?

@demiankatz, I agree with what you suggested here. That makes sense to me.

@LlGC-mag, @demiankatz, I'm a bit curious about this config option, "saveUserSettings". Does it refer to the settings tab? I tried changing the locale to something else, but when I set this config to true, it doesn't save my selected locale and instead defaults back to English.

Also;
termsOfUseEnabled", "seeAlsoEnabled" – I'm not sure what these config options are supposed to do when set to true. I tried using different sample manifests, but I didn’t notice any visible changes or results. I'm still testing the rest of it and using Saira's config doc to better understand the purpose of the remaining configurations.

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

termsOfUseEnabled controls whether a "terms of use" link appears in the footer of the Download and Share dialogs when a required statement is present. You can see this in the "Bride of the Tomb" manifest from Villanova, for example.

isSeeAlsoEnabled seems to be orphaned -- there's a method to retrieve the setting, but it's not used anywhere as far as I can tell.

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

termsOfUseEnabled controls whether a "terms of use" link appears in the footer of the Download and Share dialogs when a required statement is present. You can see this in the "Bride of the Tomb" manifest from Villanova, for example.

isSeeAlsoEnabled seems to be orphaned -- there's a method to retrieve the setting, but it's not used anywhere as far as I can tell.

I appreciate you taking the time to provide a detailed explanation.

I've tested most of the configuration options, and I wanted to highlight a few that left me a bit confused during testing, as it wasn’t always clear what behaviour to expect when applying them:

• theme
• tokenStorage
• pessimisticAccessControl
• multiSelectionMimeType
• metrics
• confinedImageSize
• currentViewDisabledPercentage

It would be helpful to have some clarification or examples on what these configurations are intended to do, especially when defined. Most of the commonly used options, particularly those related to the footerPanel, worked as expected, which is great.

This is a broad and complex area, and we can't expect everything to be perfect right away. What I really appreciate about this PR is that it enables manual adjustment and testing of config options, which will be valuable going forward.

From my side, I'm happy for this PR to be merged. That said, others are more than welcome to add their input in case I've missed anything or didn’t cover enough in testing.

Thanks again @LlGC-mag and @demiankatz for the work you’ve both put into this!

@LanieOkorodudu, there is a checklist of settings that need better documentation on #1419. Several of the settings you mentioned were already on there, and I have added the others you mentioned to the list so we remember to investigate them further.

After another look through the code there are some improvements I can see but nothing urgent enough to not merge it at this point if you're happy with the code and functionality?

Thanks @demiankatz - those final bits of whitespace should be gone now and hopefully the testers won't find anything major!

@demiankatz Thanks Demian - moved to ready for review now

Thanks, @LlGC-mag, I have merged this!