Adds a PHUI class for display images on a center point, with or without a mask.
Details
- Reviewers
epriestley - Commits
- Restricted Diffusion Commit
rPca801c7ad49a: PHUIImageMask
I am bad a math, so like, check that for me please. I tested using Photoshop. Class may need tweaked depending how we store the inline-comment coords.
Diff Detail
- Repository
- rP Phabricator
- Branch
- phui-image-mask
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 1212 Build 1212: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
My rough plan here is:
- Replace the thumbnail in Inline Comment (Transactions) with this UI.
- Maybe something with a Hovercard instead of a Dialog on the actual image. [Image/Person/Comment] (does that work on mobile?)
While I don't think it's necessarily worth changing, it might be a little cleaner to have an API like:
centerViewOnPoint(x, y) centerViewOnMask(top, left, bottom, right)
..and you do one or the other. The storage format for inlines is (top, left, bottom, right), particularly. We can clean that up later, though, if it ends up being a pain.
Using a border to do this is really clever, I hadn't thought of that.
This looks really good in the simple case, but I think it's a bit confusing when the mask is much taller, wider, or wider-and-taller than the MaskView. For example, if I select an entire panel on a mock, the MaskView will only show the center of the selection. I think this is okay (and we could scale the background as a way to dig ourselves out of this if it isn't so great), but I kind of suspect that many inlines will be on an area larger than 200x100 (i.e., dragged over an entire element).
The inline stuff is going to get huge if we deploy these, but I guess we can see how that works.
Hovercards don't work on mobile, but we could maybe make it tap-to-hovercard there.
src/applications/uiexample/examples/PHUIImageMaskExample.php | ||
---|---|---|
15 | This should use: celerity_get_resource_uri('/rsrc/image/examples/hero.png'); |
Yeah, I think in 90% of cases this will be awesome, and maybe 10% it's slightly confusing. I'm sort of going to wait for those bad experiences or try to break it a little more and solve then. Best part about being passed in here is it's easy to tweak it in one place. Like if the masked area is twice the the size of the viewport, just halve the background image. Easier to see in practice.
For inline comments in transactions, I planned on just centering the area and seeing how it feels live.
I started implementing that in this diff last night, but got stuck. Could use a hand if you have time.
src/applications/pholio/view/PholioTransactionView.php | ||
---|---|---|
93 ↗ | (On Diff #23068) | I could use a hint here, could see quite how to get the 'BestFileURL' or whatever into the comment object. |
Probably need to do something like:
- In PholioMockViewController, near line 101, call setMock($mock) on the PholioTransactionView.
- In PholioTransactionView, add a $mock property, and setMock($mock) and getMock().
- In PholioTransactionView->renderInlineContent(), do something like this:
$mock = $this->getMock(); $images = $mock->getAllImages(); $images = mpull($images, null, 'getID');
$image = idx($images, $comment->getImageID()); if (!$image) { return "bad data in database"; }
$file = $image->getFile(); if (!$file->isViewableImage()) { return "this is a PDF or something"; }
$image_uri = $file->getBestURI(); // ...
Awesome, thanks you.
I need to tweak the maths somewhere, probably not accounting for padding
Just an update for you to play with in SB if you're interested. I still need to build out the image error states.
lemme know if I mucked anything up, this feels pretty good in my sandbox. next would be to add it to the comment dialog.