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

Adds progress bar for loading images #885

Closed
wants to merge 1 commit into from
Closed

Adds progress bar for loading images #885

wants to merge 1 commit into from

Conversation

sdellis
Copy link
Member

@sdellis sdellis commented Mar 9, 2018

fixes #762

@sdellis sdellis added the Review label Mar 9, 2018
var total = this.thumbnails.length
this.numLoaded = this.numLoaded + 1
this.pctLoaded = this.numLoaded * 100 / total
console.log( 'image is ' + result + ' for ' + image.img.src )
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to leave the console logs?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, nice catch.

@sdellis
Copy link
Member Author

sdellis commented Mar 9, 2018

I am going to remove the overlay, but keep the progress bar in the gallery (just make it thin and out of the way). I still think it's helpful to see when all the images have loaded, but it will not prevent one from working with the filemanager before all the images are loaded.

Let's keep this PR open while I work on it. The new focus will be on a loading indicator for large manifests, not the images. The original issue was worded clearly, but since I've only been working with small manifests in dev, I mis-focused on the images which is what were taking a while to load for me.

@sdellis sdellis added in progress and removed Review labels Mar 9, 2018
@sdellis
Copy link
Member Author

sdellis commented Mar 9, 2018

In order to test a large manifest, I tried seeding works with lots of images by upping the number of files in this line to 1000:
https://github.com/pulibrary/figgy/blob/master/db/seeds.rb#L13

It took a long time, but I only got 7 files with 1 image each.

I'm going to need some guidance on how to do this come Monday.
Also getting some Valkyrie failed tests in CircleCI, unrelated to this work. NVM - rebased with master.

adds overlay

fixes #762

WIP - fixes images loading progress bar, still needs manifest loader
@sdellis
Copy link
Member Author

sdellis commented Mar 14, 2018

@tpendragon looking at this more closely and it seems that the manifest generation (over 10 seconds in my example case) is actually blocking the Rails layout/view rendering, so a loader/spinner does not appear while the user is waiting for the view to render. If we can render the manifest asynchronously, or after the view renders, we may be able to address this with a loader.

@tpendragon
Copy link
Contributor

@sdellis I'm confused. The file manager requests the manifest right? The only reason I can see that it would stop the rendering is if the file manager either isn't using ajax or the web server has one thread.

sdellis added a commit that referenced this pull request Mar 14, 2018
@sdellis sdellis closed this Mar 14, 2018
@eliotjordan eliotjordan deleted the issue-762 branch February 19, 2019 18:45
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.

VUE File Manager should show "loading" icon or similar while it's waiting for IIIF Manifest
2 participants