Skip to content

Initial support for thumbnails for existing files.#88

Merged
trnstlntk merged 2 commits intomasterfrom
58-thumbnails-for-existing-files
Mar 8, 2023
Merged

Initial support for thumbnails for existing files.#88
trnstlntk merged 2 commits intomasterfrom
58-thumbnails-for-existing-files

Conversation

@wetneb
Copy link
Copy Markdown
Member

@wetneb wetneb commented Dec 2, 2022

Closes #58.

This changes requires to run OpenRefine from the wetneb:5154-extension-point-for-cell-rendering branch (which corresponds to the PR OpenRefine/OpenRefine#5312).
The aim is that this PR is merged before 3.7, so that the extension can rely on it to implement the thumbnails.

Changes to the thumbnails themselves can then be made independently of OpenRefine's release cycle.

Since this PR is only about existing files, I did not add an explicit toggle to turn thumbnails on and off - they are just enabled by default for all cells that are reconciled to a media file on a Wikibase instance.

Thumbnails are only generated for file types which have thumbnailing enabled in MediaWiki (roughly, images and videos).

One thing I would probably still add is a waiting spinner when the full screen image is being loaded. It would also be worth checking the thumbnails are requested in a way that maximizes the chances of cache hits (which I am not sure I got right so far).

Screenshot

image

@trnstlntk
Copy link
Copy Markdown
Contributor

trnstlntk commented Dec 6, 2022

I tested it on a variety of files, and it works excellently. It indeed has that 💫 wow 💫 factor! I get thumbnails for images, videos and PDFs. In some cases (I saw it with a PDF and a video) the pop-up loads slowly indeed (is that due to file size?) and a spinner would indeed make sense to indicate that something is happening.

The size of the thumbnails is great IMO. Maybe other users will have different opinions but I think it's a good size to be able to discern files well without overwhelming.

I can imagine that a toggle function will be useful to add later, but this is absolutely great for now.

Comment thread module/scripts/project/thumbnail-renderer.js
@trnstlntk
Copy link
Copy Markdown
Contributor

@wetneb I noticed that this PR hasn't been merged "back in the days" (I can't recall why?) - but thumbnails do work in the Commons extension, except for this little annoying bug here: #89

Can you recall why this would still be open?

Thanks for digging into your memory :-)

@trnstlntk
Copy link
Copy Markdown
Contributor

@wetneb I noticed that this PR hasn't been merged "back in the days" (I can't recall why?) - but thumbnails do work in the Commons extension, except for this little annoying bug here: #89

Can you recall why this would still be open?

Thanks for digging into your memory :-)

If you prefer not to dig into this anymore (which is fine), I'll take the freedom to close this PR as not done, to reduce confusion.

@wetneb
Copy link
Copy Markdown
Member Author

wetneb commented Feb 28, 2023

I think you do need to merge this PR - I do not think thumbnail support works without it. Could it be that the version of the Commons extension you are using is generated from this PR?

So I would recommend merging this and releasing a new version of the extension.

@trnstlntk
Copy link
Copy Markdown
Contributor

I think you do need to merge this PR - I do not think thumbnail support works without it. Could it be that the version of the Commons extension you are using is generated from this PR?

So I would recommend merging this and releasing a new version of the extension.

Thanks, you're entirely correct! I now feel very stupid 🙈 - I was indeed working from this PR myself.

I notice we released 0.1.0 without thumbnail support (it was also not included in the release notes). I prefer for someone to continue adjusting this existing PR to fix the #89 bug, and also someone else to release the new version. I did the previous release myself but found it very difficult, I had absolutely no idea what I was doing except for following instructions blindly. IMO it's a job for someone with stronger technical skills and familiarity with Git/GitHub processes.

I'll see how I can adjust the bounty to include these requirements.

Copy link
Copy Markdown
Contributor

@trnstlntk trnstlntk left a comment

Choose a reason for hiding this comment

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

After quite a bit more testing of this PR, I have noticed that it doesn't support all file extensions on Wikimedia Commons, see issue #89 (capitalized file names)

I suggest to update this PR accordingly, then merge, and do a new release of the extension.

@trnstlntk trnstlntk merged commit 40d8b4c into master Mar 8, 2023
@trnstlntk trnstlntk deleted the 58-thumbnails-for-existing-files branch March 8, 2023 12:27
@vitaly-zdanevich
Copy link
Copy Markdown

But I have previews only on mouse hover:
image

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.

Thumbnail previews of media files available on Wikimedia Commons

4 participants