embedded iframe for videos #34

summer-cook · 2023-04-28T13:15:51Z

Story

SoftServ will customize the ability for YouTube videos to appear embedded on a work show page. Videos will not be played back in Universal Viewer.

Expected Behavior Before Changes

all works that should show a UV show them
users would only see either the uv viewer or default image

Expected Behavior After Changes

If a work has a valid video_embed link saved in its metadata, the work show page will show that video in an iframe.
If a work does not have a valid video_embed link saved in its metadata, but should show the universal viewer because it has files, it shows the viewer.
If a work does not have either video_embed metadata or files to show in the viewer, it shows the default image.

Screenshots / Video

Working youtube player

Working vimeo player

Form with error message for invalid url entry

Acceptance

Users should be able to add a video embed link from the work-edit page in the dashboard
Users can see help text explaining the format for the links, as well as more in depth directions for how to find the link in the project readme
The field should support embed links for either youtube or vimeo links
The field should not support urls that are not valid youtube or vimeo links in the formats provided in the help text.
On the work show page, the user should be able to see the embedded video if they add a link
If there is no link added, the work show page will show the UV or the default image (behavior would be the same and prior to this implementation)
All 3 show page (cultural, scholarly, default) themes should support the video-embed viewer

Note

ability to import the video-embed links with bulkrax will be done in the bulkrax configuration ticket
this still needs to be added to the 2 other worktypes which are in flight

…-embedded-iframe

summer-cook · 2023-05-02T18:46:39Z

app/presenters/hyku/work_show_presenter.rb

    private
+      def extract_video_embed_presence


I had to set this up this way because when i was trying to use solr_document above the private methods, it was giving me an error that solr_document was private.

I also got a similar error (screenshot below) when trying to use. presenter.solr_document directly in the view. only using it /calling the private method in the presenter itself seems to have fixed that issue

summer-cook · 2023-05-02T18:50:44Z

app/views/layouts/hyrax/dashboard.html.erb

+        <%# OVERRIDE here to hide broken flash messages on the dashboard edit work pages. %>
+        <%# regex includes only the paths for works since this is the only known place flash messages are broken. %>
+        <% if !controller.request.env['REQUEST_PATH'].include?('concern') %>
+          <%= render '/flash_msg' %>


although we originally said to remove the whole partial, that didn't really make sense since this is the flash message that is being used everywhere in the dashboard.
It is not necessarily broken everywhere in the dashboard, but it is for validations on works.
This conditional only removes the flash message if the path includes "concern" which is only on works

labradford · 2023-05-02T19:43:47Z

app/views/hyrax/base/show.html.erb

-            <%= render 'representative_media', presenter: @presenter, viewer: false unless @presenter.iiif_viewer? %>
+            <% if !@presenter.video_embed_viewer? %>
+              <%= render 'representative_media', presenter: @presenter, viewer: false unless @presenter.universal_viewer? %>
+            <% end %>


Suggested change

<% end %>

<%= render 'representative_media', presenter: @presenter, viewer: false unless @presenter.universal_viewer? && !@presenter.video_embed_viewer? %>

Would this work making it a single line?

Or maybe wrap the whole block like:

Suggested change

<% end %>

<% if @presenter.video_embed_viewer? %>

<%= render 'video_embed_viewer', presenter: @presenter %>

<% else %>

<% if @presenter.iiif_viewer? %>

<div class="col-sm-12">

<%= render 'representative_media', presenter: @presenter, viewer: true %>

</div>

<% else %>

<div class="col-sm-3 text-center">

<%= render 'representative_media', presenter: @presenter, viewer: false %>

</div>

<% end %>

<% end %>

I am going to change it to this, its way easier to read

and maybe this is contribute back-able so it would be less confusing in hyku

labradford · 2023-05-02T19:49:17Z

app/views/themes/cultural_show/hyrax/base/show.html.erb

+          <% if @presenter.video_embed_viewer? %>
+            <%= render 'video_embed_viewer', presenter: @presenter %>
+          <% end %>
+          <% if @presenter.universal_viewer? & !@presenter.video_embed_viewer? %>


maybe just use and if/else statement here

labradford

My comments are just suggestions, I think it's ok to approve. I do think the conditional rendering of the UV/Video/Representative media is a bit convoluted.

Great job remembering the themes!

summer-cook · 2023-05-02T20:09:33Z

@labradford

My comments are just suggestions, I think it's ok to approve. I do think the conditional rendering of the UV/Video/Representative media is a bit convoluted.

Great job remembering the themes!

I know, originally i wanted to just put it in the representative_media partial and be done with it, but then that ALSO required changes to the show page since there are viewer related conditionals in there too. I will test out your suggestions though, thank you!

summer-cook · 2023-05-02T20:55:31Z

@labradford i just pushed up the show page views again with your suggestions, but had to do some weird things with the default one to get the columns to work.
let me know if you want to take a look again

ShanaLMoore · 2023-12-05T22:46:33Z

app/views/layouts/hyrax/dashboard.html.erb

+        <%# OVERRIDE here to hide broken flash messages on the dashboard edit work pages. %>
+        <%# regex includes only the paths for works since this is the only known place flash messages are broken. %>
+          <%= render '/flash_msg' %>
+          <%#= raise 'hell' %>


oops, this should be removed!

add video embed field and get it saving correctly

bd49e15

summer-cook closed this Apr 28, 2023

summer-cook reopened this Apr 28, 2023

summer-cook added 6 commits April 28, 2023 12:04

Merge branch 'main' of github.com:scientist-softserv/atla-hyku into 9…

fbc2a2d

…-embedded-iframe

add help text

5b3c09d

update readme, styles, and yml

a221372

commit the show pages just in case

cb3b65e

make sure generic work form is set up

22959b3

add comments and update views for the messed up validation messages

8c2c24e

summer-cook commented May 2, 2023

View reviewed changes

fix rubocop

9ff3179

summer-cook marked this pull request as ready for review May 2, 2023 19:13

summer-cook mentioned this pull request May 2, 2023

Add video embed property to 2 other worktypes #41

Open

1 task

add mapping to csv parser

01e3605

labradford requested changes May 2, 2023

View reviewed changes

use && not &

c35b172

labradford approved these changes May 2, 2023

View reviewed changes

refactor show pages to make more readable

fb7fb77

readme change

baaca22

labradford approved these changes May 2, 2023

View reviewed changes

summer-cook added 4 commits May 3, 2023 10:35

fix admin issue causing test to fail

4e96de9

merge conflicts

3021b67

add nil check

1dd40b8

remove flash message changes altogether

6ae8dcf

summer-cook merged commit bf7eca0 into main May 4, 2023

summer-cook mentioned this pull request May 9, 2023

Add :video_embed to paper or report worktype #51

Merged

1 task

aahurford mentioned this pull request Dec 1, 2023

Embed codes for media (see Atla instance) notch8/palni-palci#911

Closed

19 tasks

ShanaLMoore reviewed Dec 5, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

embedded iframe for videos #34

embedded iframe for videos #34

summer-cook commented Apr 28, 2023 •

edited

Loading

summer-cook May 2, 2023 •

edited

Loading

summer-cook May 2, 2023

labradford May 2, 2023

labradford May 2, 2023

summer-cook May 2, 2023

summer-cook May 2, 2023

labradford May 2, 2023

labradford left a comment

summer-cook commented May 2, 2023

summer-cook commented May 2, 2023

ShanaLMoore Dec 5, 2023

	<% end %>
	<%= render 'representative_media', presenter: @presenter, viewer: false unless @presenter.universal_viewer? && !@presenter.video_embed_viewer? %>

-            <% end %>
+<% if @presenter.video_embed_viewer? %>
+  <%= render 'video_embed_viewer', presenter: @presenter %>
+<% else %>
+  <% if @presenter.iiif_viewer? %>
+    <div class="col-sm-12">
+      <%= render 'representative_media', presenter: @presenter, viewer: true %>
+    </div>
+  <% else %>
+    <div class="col-sm-3 text-center">
+      <%= render 'representative_media', presenter: @presenter, viewer: false %>
+    </div>
+  <% end %>
+<% end %>

embedded iframe for videos #34

embedded iframe for videos #34

Conversation

summer-cook commented Apr 28, 2023 • edited Loading

Story

Related

Expected Behavior Before Changes

Expected Behavior After Changes

Screenshots / Video

Acceptance

Note

summer-cook May 2, 2023 • edited Loading

Choose a reason for hiding this comment

summer-cook May 2, 2023

Choose a reason for hiding this comment

labradford May 2, 2023

Choose a reason for hiding this comment

labradford May 2, 2023

Choose a reason for hiding this comment

summer-cook May 2, 2023

Choose a reason for hiding this comment

summer-cook May 2, 2023

Choose a reason for hiding this comment

labradford May 2, 2023

Choose a reason for hiding this comment

labradford left a comment

Choose a reason for hiding this comment

summer-cook commented May 2, 2023

summer-cook commented May 2, 2023

ShanaLMoore Dec 5, 2023

Choose a reason for hiding this comment

summer-cook commented Apr 28, 2023 •

edited

Loading

summer-cook May 2, 2023 •

edited

Loading