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

🎁 Add video embed to Generic Work #922

Merged
merged 2 commits into from
Dec 6, 2023
Merged

Conversation

ShanaLMoore
Copy link
Contributor

@ShanaLMoore ShanaLMoore commented Dec 6, 2023

This commit allows videos to be embedded into a Generic Work. When the value is present, an embedded video should display.

related:

Issue:

TODO

This PR is already large. I may create sub task for the tickets in attempts to keep this epic of work small. This PR only handles embedding the video for Generic Works, base theme, and in English only.

  • Run translations
  • Add video embed options to other work types
  • Add embedded video to other theme's show pages

Expected Behavior Before Changes

This feature previously did not exist.

Expected Behavior After Changes

A user can create a Generic Work and embed a video. If the value exists, the embedded video will display in the iframe of the show page and the user is able to play it.

Screenshot 2023-12-05 at 15-39-30 embed

When there is an error on the form (ie: an invalid embed link), messages appear to say so:

Screenshot 2023-12-05 at 16-17-09 embed __ Work fb7583b6-bf66-478a-b0c9-dad657fd60d4 __ Dev
Screenshot 2023-12-05 at 16-17-59 embed __ Work fb7583b6-bf66-478a-b0c9-dad657fd60d4 __ Dev

Notes

This commit allows videos to be embedded into a Generic Work. When the value is present, an embedded video should display.

related:
- https://github.com/scientist-softserv/atla-hyku/pull/34/files

Issue:
- #911

private

def extract_video_embed_presence
solr_document[:video_embed_tesim].present?
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that this is multi-value, you may want to consider the following:

def extract_video_embed_presence
  solr_document[:video_embed_tesim]&.first&.present?
end

This will mirror the eventual call in the partial. Because [""].present? will be true.

Co-authored-by: Jeremy Friesen <jeremy.n.friesen@gmail.com>
@ShanaLMoore ShanaLMoore marked this pull request as ready for review December 6, 2023 15:55
@ShanaLMoore ShanaLMoore changed the title 🚧 Add video embed to Generic Work 🎁 Add video embed to Generic Work Dec 6, 2023
@ShanaLMoore ShanaLMoore merged commit baef5bf into main Dec 6, 2023
6 of 7 checks passed
@ShanaLMoore ShanaLMoore deleted the i911-embed-video-links branch December 6, 2023 16:29
jeremyf added a commit to samvera/hyku that referenced this pull request Dec 19, 2023
jeremyf added a commit to samvera/hyku that referenced this pull request Dec 19, 2023
jeremyf added a commit to samvera/hyku that referenced this pull request Dec 19, 2023
jeremyf added a commit to samvera/hyku that referenced this pull request Dec 19, 2023
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.

2 participants