Extending the thumbnail label to reveal more text #758

[Saira-A]

Issue #758

[codesandbox[bot]]

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders
Open Preview

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
universalviewer	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 18, 2024 11:43pm

[codesandbox-ci[bot]]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

[edsilv]

it looks like the label span isn't filling the whole width of its containing element? Might make things easier to read. usually settings with checkboxes associated are located in the settings dialogue (cog icon, top right)

[demiankatz]

@Saira-A, I also think some more work may be needed to support this functionality in extensions other than OpenSeadragon. At least, when I tested this with a multi-PDF manifest, the labels did not expand. You can see this here:

https://universalviewer-git-fork-saira-a-758-mnemoscene.vercel.app/#?xywh=-1210%2C-424%2C10923%2C8463&iiifManifestId=https%3A%2F%2Fdigital.library.villanova.edu%2FItem%2Fvudl%3A294631%2FManifest

[Saira-A]

@Saira-A, I also think some more work may be needed to support this functionality in extensions other than OpenSeadragon. At least, when I tested this with a multi-PDF manifest, the labels did not expand. You can see this here:

https://universalviewer-git-fork-saira-a-758-mnemoscene.vercel.app/#?xywh=-1210%2C-424%2C10923%2C8463&iiifManifestId=https%3A%2F%2Fdigital.library.villanova.edu%2FItem%2Fvudl%3A294631%2FManifest

I've done it so that when thumbnails are one up, they're extended by default as there's more space for them anyway so now the checkbox only affects two-up view

[Saira-A]

See below for a few more thoughts and minor nitpicks.

Beyond that, though, there seems to be a bigger issue. I definitely like what you've done with 1-up labels, but it seems that 2-up mode has somehow gotten broken in the process. When I look at the original test case, it comes up in 1-up mode and I don't see a way to switch it to 2-up.

Here's a link for convenience:

https://universalviewer-git-fork-saira-a-758-mnemoscene.vercel.app/#?xywh=-1538%2C0%2C11578%2C7617&iiifManifestId=https%3A%2F%2Fcollections.st-andrews.ac.uk%2F762295%2Fmanifest

I think this happened prior to my commit? I went back to the docs branch and it has the same issue, and there's no icons to switch between one and two up, yet other manifests look fine

[demiankatz]

I think this happened prior to my commit? I went back to the docs branch and it has the same issue, and there's no icons to switch between one and two up, yet other manifests look fine

Hmm, I see, this is all a bit confusing. The example manifest that was first cited when this issue was opened uses viewingHint: individuals, which disables 2-up functionality. Thus, I believe it is correct for it NOT to display 2-up mode or side-by-side thumbnails. However, if you look at the manifest in older versions of UV4, it shows 2-up thumbnails with abbreviated labels, probably due to some older bug that has been subsequently fixed. The odd thing is that I was able to see 2-up thumbnails with this manifest quite recently, since I used it for testing last week and was able to see your behavior applied.

In any case, regardless of what has changed recently or why, I think we need a new example manifest with viewingHint: paged and long page descriptions so that we can realistically test your work. Do you happen to have such an example that you could share?

[Saira-A]

I think this happened prior to my commit? I went back to the docs branch and it has the same issue, and there's no icons to switch between one and two up, yet other manifests look fine

Hmm, I see, this is all a bit confusing. The example manifest that was first cited when this issue was opened uses viewingHint: individuals, which disables 2-up functionality. Thus, I believe it is correct for it NOT to display 2-up mode or side-by-side thumbnails. However, if you look at the manifest in older versions of UV4, it shows 2-up thumbnails with abbreviated labels, probably due to some older bug that has been subsequently fixed. The odd thing is that I was able to see 2-up thumbnails with this manifest quite recently, since I used it for testing last week and was able to see your behavior applied.

In any case, regardless of what has changed recently or why, I think we need a new example manifest with viewingHint: paged and long page descriptions so that we can realistically test your work. Do you happen to have such an example that you could share?

will this do? https://iiif.bodleian.ox.ac.uk/examples/mushaf4.json

[demiankatz]

will this do? https://iiif.bodleian.ox.ac.uk/examples/mushaf4.json

Yes, that's great! Here's a convenient link to the test environment if anyone else wants to experiment with it:

https://universalviewer-git-fork-saira-a-758-mnemoscene.vercel.app/#?xywh=-506%2C-1%2C8226%2C5412&iiifManifestId=https%3A%2F%2Fiiif.bodleian.ox.ac.uk%2Fexamples%2Fmushaf4.json

I think for the most part this looks very good; however, there's a strange issue in the gallery view where the rows of images are uneven. I think this may be caused by some labels wrapping to two lines while others take up only a single line. We might need to figure out a way to address that... Here's what I'm seeing:

[Saira-A]

will this do? https://iiif.bodleian.ox.ac.uk/examples/mushaf4.json

Yes, that's great! Here's a convenient link to the test environment if anyone else wants to experiment with it:

https://universalviewer-git-fork-saira-a-758-mnemoscene.vercel.app/#?xywh=-506%2C-1%2C8226%2C5412&iiifManifestId=https%3A%2F%2Fiiif.bodleian.ox.ac.uk%2Fexamples%2Fmushaf4.json

I think for the most part this looks very good; however, there's a strange issue in the gallery view where the rows of images are uneven. I think this may be caused by some labels wrapping to two lines while others take up only a single line. We might need to figure out a way to address that... Here's what I'm seeing:

Changing the galleryview css to this: .galleryView { .main { .thumbs { .thumb { width: 140px; .info { .label { white-space: break-spaces; } } } } } } } } }

fixes that issue but the extra space between thumbnails means that one less fits per row:

and if I decrease the width any more the original issue comes back.

Doing it like this instead keeps the same amount of space as before: .galleryView { .main { .thumbs { display: flex; flex-wrap: wrap; .thumb { .info { .label { white-space: break-spaces; } } } } } } } } } but ruins right-to-left:

[demiankatz]

@Saira-A, I suspect that the issue is dependent on whether the first thumbnail in the row has shorter label text than subsequent thumbnails, so the width doesn't fix or break it but just happens in this instance to move things around in such a way that the behavior changes.

I've heard that CSS grid can be helpful for this sort of problem, but I haven't had a chance to learn about it yet (I spend much more of my time in back-end code these days). I wonder if that might be worth investigating as a solution....

[Saira-A]

@Saira-A, I suspect that the issue is dependent on whether the first thumbnail in the row has shorter label text than subsequent thumbnails, so the width doesn't fix or break it but just happens in this instance to move things around in such a way that the behavior changes.

I've heard that CSS grid can be helpful for this sort of problem, but I haven't had a chance to learn about it yet (I spend much more of my time in back-end code these days). I wonder if that might be worth investigating as a solution....

Grid fixed the layout issue but has the same problem as flex with right-to-left not working. I instead added a minimum and maximum height of the same value to the label which gives enough room for two lines of label text before cutting anything further off

[demiankatz]

Thanks, @Saira-A -- while that may not be the best theoretical solution to the problem, it seems like a reasonable pragmatic approach, at least for the moment!

I think this PR will be a great topic of discussion for the next Community Call; I think we should merge it soon, but we should decide as a community what steps to take to be confident that we're ready to move forward and aren't missing anything.

[demiankatz]

Thanks for resolving the conflicts, @Saira-A! I see that you removed the tests; did they stop working for you? I think it might be worth trying to revive them, though we might need to figure out the most appropriate place to put them. Maybe something to discuss on a future dev call.

[Saira-A]

Thanks for resolving the conflicts, @Saira-A! I see that you removed the tests; did they stop working for you? I think it might be worth trying to revive them, though we might need to figure out the most appropriate place to put them. Maybe something to discuss on a future dev call.

I removed the tests since the test suite doesn't run on the laptop I'm using and there have been other changes to the test file so I can't tell if they still work, but I still have them saved and we're getting new laptops soon so I'll try to put them back then

[demiankatz]

I removed the tests since the test suite doesn't run on the laptop I'm using and there have been other changes to the test file so I can't tell if they still work, but I still have them saved and we're getting new laptops soon so I'll try to put them back then

Makes sense! Let me know once everything is ready and I can help test (and we can perhaps discuss the best placed to put the tests, since I seem to recall that when we looked at this before, we found there were a few possible approaches supported by the current setup... and it would probably be ideal to have a separate test file for each discrete feature).