Show image preview thumbnails on Authored Files
Needs ReviewPublic
Actions

Authored by aklapper on Sat, May 17, 13:10.

Details

Reviewers

None

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T16039: Show image preview thumbnails on your Authored Files

Summary

When going through the list of image files which you yourself uploaded it is convenient to have a visual representation.
Users often do not spend time to enter descriptive filenames (which are not really used afterwards in the UI anyway) to differentiate.

As this can be a performance hole, limit to the viewer's authored files view only.

Closes T16039

Test Plan

Be logged in, upload some image files, go to http://phorge.localhost/file/query/authored/, see thumbnails
Go to http://phorge.localhost/file/query/all/ or be logged out and see no thumbnails

Diff Detail

Repository

rP Phorge

Branch

T16039fileUploadImagePreviews (branched from master)

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 2058
Build 2058: arc lint + arc unit

Event Timeline

aklapper created this revision.Sat, May 17, 13:10

Owners added a reviewer: O1: Blessed Committers.Sat, May 17, 13:10

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptSat, May 17, 13:10

aklapper requested review of this revision.Sat, May 17, 13:10

Harbormaster completed remote builds in B1985: Diff 3059.Sat, May 17, 13:10

valerio.bozzolan added inline comments.Wed, Jun 4, 13:03

src/applications/files/query/PhabricatorFileSearchEngine.php
146–149	Let's always define the variable
170	Let's just read the boolean. It's strange to have it defined or not
172–173	Bonus point 1: Probably the `$xform = PhabricatorFileTransform::getTransformByKey(` definition should be moved outside the loop as micro-optimization. Bonus point 2: And defined only if `$authored_view` is true.

A rather boring git rebase master with no particular exciting new features

Harbormaster completed remote builds in B2056: Diff 3152.Wed, Jun 4, 16:58

Do as Valerio says

Harbormaster completed remote builds in B2057: Diff 3153.Wed, Jun 4, 17:04

I'm still clueless if this could be more optimized somehow given my lack of understanding of the thumbnail software stack. Just saying.

valerio.bozzolan added inline comments.Wed, Jun 4, 17:30

src/applications/files/query/PhabricatorFileSearchEngine.php
146–149	this 👍
171–172	aaaand now, as I was mumbling aloud, maybe super-nice to move the `$xform =` definition out the foreach, so, we can move this up (extra extra bonus bonus point: once it's up, we can find the `$xform` only if it's necessary)

aaaand now, as I was mumbling aloud, maybe super-nice to move the $xform = definition out the foreach, so, we can move this up

(extra extra bonus bonus point: once it's up, we can find the $xform only if it's necessary)

Argh. Yes you wrote that before and I should have read it more carefully. Sorry! Done now. I think.

Harbormaster completed remote builds in B2058: Diff 3154.Wed, Jun 4, 18:04