Skip to content
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

♻️ Account for observed customizations #2113

Closed
wants to merge 1 commit into from

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented Dec 22, 2023

Dear reviewer, put on your reading glasses.

This commit looks at the newly refactored
Hyrax::IiifAv::DisplaysContentDecorator as well as the current state of PALS's Hyrax::IiifAv::DisplaysContentDecorator and attempts to account for the variances between the two by introducing configurations.

Yes, we could port this to the hyrax-iiif_av gem, but for now that would not solve the underlying issue of how we've been handling things.

# @return [String] the fully qualified URL.
#
# @example
# # The below example will build a URL taht will download directly from Hyrax as the
Copy link
Collaborator

Choose a reason for hiding this comment

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

just a typo for the word that

@jeremyf jeremyf force-pushed the hyrax-5-upgrade-refactor-for-pals branch 2 times, most recently from 91cb83e to f88468c Compare December 22, 2023 16:08

##
# @!attribute iiif_video_labels_and_mime_types [r|w]
# @return [Hash<String,String>] Hash of valid vidoe labels and their mime types.
Copy link
Collaborator

Choose a reason for hiding this comment

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

just a typo video

Copy link
Collaborator

@kirkkwang kirkkwang left a comment

Choose a reason for hiding this comment

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

Cool, i think it looks good

@jeremyf jeremyf force-pushed the hyrax-5-upgrade-refactor-for-pals branch from f88468c to eb99895 Compare December 22, 2023 16:37
@jeremyf jeremyf added the patch-ver for release notes label Dec 22, 2023
Dear reviewer, put on your reading glasses.

This commit looks at the newly refactored
`Hyrax::IiifAv::DisplaysContentDecorator` as well as the current state
of [PALS's Hyrax::IiifAv::DisplaysContentDecorator][1] and attempts to
account for the variances between the two by introducing configurations.

Yes, we could port this to the hyrax-iiif_av gem, but for now that would
not solve the underlying issue of how we've been handling things.

[1]: https://github.com/scientist-softserv/palni-palci/blob/8754556c0225ce9f04674c1ffac6403586fd65f4/app/presenters/concerns/hyrax/iiif_av/displays_content_decorator.rb
@jeremyf jeremyf force-pushed the hyrax-5-upgrade-refactor-for-pals branch from eb99895 to b18a03b Compare December 22, 2023 16:44
@jeremyf jeremyf closed this Dec 22, 2023
@jeremyf jeremyf deleted the hyrax-5-upgrade-refactor-for-pals branch December 22, 2023 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch-ver for release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants