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.

Sign up for GitHub

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

Allow location restricted files to show in document viewer content window #2752

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+4 −4
Open

Allow location restricted files to show in document viewer content window #2752

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9a53a67
allowing location restricted files to show in the document viewer con…
hudajkhan Mar 5, 2025
409f142
hiding login panel by default to handle clicking between different files
hudajkhan Mar 6, 2025
5c937b9
removing default hiding of login panel as this will be taken care of …
hudajkhan Mar 6, 2025
aef6581
adding extra line back
hudajkhan Mar 6, 2025
a705c39
formatting, ensuring logic fixed for companion window rendering of co…
hudajkhan Mar 6, 2025
44ee824
fixing rubocop syntax
hudajkhan Mar 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/components/companion_windows/content_list_component.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
data-controller="content-list"
data-action="thumbnails-found@window->content-list#drawThumbnails"
role="tablist">
<%= render Document::ContentListItemComponent.with_collection(viewer.purl_object.downloadable_files, viewer:) if document_viewer? %>
<%= render Document::ContentListItemComponent.with_collection(viewer.purl_object.all_resource_files.reject(&:no_download?), viewer:) if document_viewer? %>
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this now show thumbnail files for media, where previously it wouldn't?

Copy link
Contributor Author

@hudajkhan hudajkhan Mar 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This case seems limited only to the document viewer (because it says if document_viewer? at the end) so I don't think so. I haven't tested it.

Copy link
Contributor

Choose a reason for hiding this comment

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

This component is shared between all of the viewers, so a change here affects them all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I interpreted the "if document_viewer" portion to mean this should only render for document viewer and not media viewer. I will try it out with a media viewer example to confirm.

Copy link
Contributor

Choose a reason for hiding this comment

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

Huda is correct. The sidebar is populated by the media controller for media.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, sorry. I missed that part. Please disregard.

</ul>
<% end %>
<% end %>
6 changes: 3 additions & 3 deletions app/components/companion_windows_component.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def initialize(viewer:, stimulus_controller: '')
attr_reader :viewer

delegate :purl_object, :display_header?, to: :viewer
delegate :downloadable_files, :druid, :title, :iiif_v3_manifest_url, to: :purl_object
delegate :downloadable_files, :all_resource_files, :druid, :title, :iiif_v3_manifest_url, to: :purl_object

def media_viewer?
viewer.instance_of?(::Embed::Viewer::Media)
Expand All @@ -30,8 +30,8 @@ def document_viewer?
end

def render_content_list_panel?
# for Document viewer, do not render the content panel if there is just one downloadable file in the object
media_viewer? || (document_viewer? && downloadable_files.size > 1)
# for Document viewer, do not render the content panel if there only one file that isn't no_download?
media_viewer? || (document_viewer? && all_resource_files.count { |element| !element.no_download? } > 1)
end

def display_download?
Expand Down