sul-dlss / sul-embed

An oEmbed Service for Stanford University Libraries
Other
19 stars 6 forks source link

Support for viewing objects with file hiearchy #1380

Closed peetucket closed 1 year ago

peetucket commented 2 years ago

We now allow for SDR objects to have files in a hierarchy. This is manifested by having filenames that can includes relative paths.

Start working on this issue by rigging up data that is very wide (lots of hierarchy / long directory & file names) to see how the embed frame looks. (See Testing section below.)

NOTE: There are many comments below, but whoever works on this should NOT need to review them, as @mjgiarlo incorporated the actionable bits into this PR description.

Scope

Testing

sul-embed has a helpful sandbox view that works when running in development: http://127.0.0.1:3000/pages/sandbox

To get it to render the embed iframe for data of HFS shape, I'd recommend looking at the public XML of an existing HFS object, e.g., https://sul-purl-stage.stanford.edu/mh023zv6595.xml and then embellishing it if needed (adding more resources, adding a mix of files with paths, nested paths, no paths, etc.). Save the XML file as baredruidhere.xml within the sul-embed public folder, and then make a copy of the file as baredruidhere in the public folder. Why all these hijinks? Then you can modify the XML as you need without any external dependencies, and the sandbox will show you how its embed iframe will render. Paste e.g. http://localhost:3000/mh023zv6595 into the URL field of the sandbox form and then click Embed

Prior Work

Here was my prior WIP. Feel free to run with it or throw it away: https://github.com/sul-dlss/sul-embed/pull/1390

Design

Current

Currently the files are listed with paths (e.g. https://sul-purl-stage.stanford.edu/dt568vs5903):

Screen Shot 2022-11-11 at 11 36 47 AM

Desired

We would instead need them to be shown with hierarchy:

image
astridu commented 1 year ago

Could we have expand/collapse controls on the nested folders?

jcoyne commented 1 year ago

Is there a limit on the maximum depth permitted?

@astridu can you say more about why we would want expand/collapse controls? I would think this would make it challenging (e.g. many interactions required) to quickly see what is in a deeply nested hierarchy.

peetucket commented 1 year ago

Is there a limit on the maximum depth permitted?

I am assuming that practically speaking, it will likely be shallow (a few levels deep), but we are not going to enforce any limits on what a user can upload. Do you agree @amyehodge and @andrewjbtw ?

@astridu can you say more about why we would want expand/collapse controls? I would think this would make it challenging (e.g. many interactions required) to quickly see what is in a deeply nested hierarchy.

I think the idea is that because it could theoretically be very nested and/or have many files at any given level, the expand/collapse controls keep the download panel from becoming unwieldy.

Though expand/contract won't help much if you have a lot files at the top level (which presumably will start out expanded by default).

jcoyne commented 1 year ago

I think the idea is that because it could theoretically be very nested and/or have many files at any given level, the expand/collapse controls keep the download panel from becoming unwieldy.

I'm suggesting that the expand/collapse controls would actually make a simple scan of what is in the item a real pain.

amyehodge commented 1 year ago

I am assuming that practically speaking, it will likely be shallow (a few levels deep), but we are not going to enforce any limits on what a user can upload. Do you agree @amyehodge and @andrewjbtw ?

Agreed

amyehodge commented 1 year ago

I'm suggesting that the expand/collapse controls would actually make a simple scan of what is in the item a real pain.

I agree. It's like in Argo when you had to manually expand each of the levels of the cocina one at a time. It's painful.

andrewjbtw commented 1 year ago

You can expand the whole Cocina at once in Argo.

andrewjbtw commented 1 year ago

I think for an example like this one from CalTech, it would be helpful to be able to expand/collapse as an option but it does look pretty nice with it fully expanded: https://data.caltech.edu/records/q2kby-f9t84

peetucket commented 1 year ago

Any performance/viewing concerns with an object with a very large number of files?

I recall working with one object in Argo that had many thousands of files, and just loading the cocina took like 30 seconds before any rendering happened. Or enough of an edge case to not be worried about now?

amyehodge commented 1 year ago

Hopefully an edge case? My concern is more around making sure you can see enough of it at one time in the viewer window to be able to keep track of what's going on. So, that's more about the formatting, font size, spacing, etc.

astridu commented 1 year ago

Is there a limit on the maximum depth permitted?

@astridu can you say more about why we would want expand/collapse controls? I would think this would make it challenging (e.g. many interactions required) to quickly see what is in a deeply nested hierarchy.

Definitely a nice to have, and as Peter said above, it gives the user the flexibility to see certain portions of the entire file structure they may be interested in while hiding parts they are not interested in. However, not critical for this phase.

mjgiarlo commented 1 year ago

We no longer consider this item blocked, but are now proposing a two-phase approach:

  1. First, implement the minimal set of changes to support reflecting hierarchy in the sul-embed UI without breaking anything. This work is unblocked now.
  2. Second, implement the snazzier design (that may have collapse/expand controls per above)
astridu commented 1 year ago

Here is an updated mock up with prioritized list of features:

  1. Display file hierarchy
  2. Display file size next to each file
  3. Display download icon with ability to download each file individually
  4. Display folder or file icon
  5. Allow user to expand/collapse each of the folders. image
mjgiarlo commented 1 year ago

I'm finding this issue a bit too involved to get my head into as tech lead. I'll drop a few notes here:

mjgiarlo commented 1 year ago

Here was my prior WIP. Feel free to run with it or throw it away: https://github.com/sul-dlss/sul-embed/pull/1390

mjgiarlo commented 1 year ago

NOTE: For now, we understand that sul-embed will display files in the same order as specified in the content metadata, and we have no plans to deviate from that. Given that H2 will lexically sort files in content metadata based on their paths, here's a mock-up showing how this might look:

image
justinlittman commented 1 year ago

A few questions:

mjgiarlo commented 1 year ago

@justinlittman 💬

A few questions:

  • What happens if the hierarchy is wider than the embed box?

Not sure. We can rig up some data that causes this to be the case to test it out.

  • The current embed box seems to have a very limited height (for example, here). Will this work with that height? Or does the height need to be increased? Does a larger height work in all of the contexts in which it is embedded?
  • Do we know how the dimensions of the embed viewer are determined? Are they controlled by sul-embed or the embed host?

AFAICR:

ggeisler commented 1 year ago

Some thoughts from a general SUL-Embed perspective:

I guess what I'm saying is that, at least for the initial rollout of file hierarchy in the viewer, maybe we should look for ways to add features that help the user deal with the more extreme file hierarchy use cases, but do so without changing the fundamental way we currently display the viewer.

mjgiarlo commented 1 year ago

@ggeisler Heartily agree! Thanks for that, Gary. I reflected this in the Scope section above: https://github.com/sul-dlss/sul-embed/issues/1380#issue-1445931300

justinlittman commented 1 year ago

Here is an XML files with hierarchy (that can be placed in public/). st521wt2240.xml.zip

(Zipped to allow GH upload.)

justinlittman commented 1 year ago

Add these to config/settings/development.yml:

valid_purl_url: <%= /http?:\/\/localhost:3000\/*/ %>
purl_url: 'http://localhost:3000'
justinlittman commented 1 year ago

image

justinlittman commented 1 year ago

Declaring victory.