-
Notifications
You must be signed in to change notification settings - Fork 15
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
Fix PHP notices - Issue 1124 #11
Conversation
@seth-shaw-unlv what about all the other settings not included in the install config which instead now have these defaults in the form? Basically everywhere you have added an Or I think all of them, at least the first 7 but then I stopped checking |
Then the config is loaded at install time and the |
@whikloj The ones I added the isset for (e.g. compositeOperation) didn't have entries in openseadragon.settings. True, adding them there would be simpler, but I didn't know what a reasonable default would be, so I opted for checking instead. Should I just change this PR to give them all blank values in the config? |
@seth-shaw-unlv Can we not add a default of |
@whikloj, probably. I'll try it once I'm done testing a different PR. |
I've added default settings to config and reverted the issets. How's it look @whikloj? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good and works as advertised.
GitHub Issue: Islandora/documentation/issues/1124 and Islandora/documentation/issues/1125
What does this Pull Request do?
Resolves PHP notices about missing settings for the viewer and settings form. Also fixes some minor form errors.
What's new?
(ie. Regeneration activity, etc.)? No
How should this be tested?
Interested parties
@kayakr, @Islandora-CLAW/committers