Page MenuHomePhabricator

PHUIImageMask
ClosedPublic

Authored by chad on Jun 17 2014, 11:59 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 24, 10:23 PM
Unknown Object (File)
Fri, Apr 19, 1:59 PM
Unknown Object (File)
Fri, Apr 12, 3:19 AM
Unknown Object (File)
Mar 4 2024, 7:37 PM
Unknown Object (File)
Feb 6 2024, 8:56 PM
Unknown Object (File)
Jan 12 2024, 2:18 PM
Unknown Object (File)
Jan 10 2024, 11:01 AM
Unknown Object (File)
Jan 7 2024, 4:17 PM
Subscribers

Details

Reviewers
epriestley
Commits
Restricted Diffusion Commit
rPca801c7ad49a: PHUIImageMask
Summary

Adds a PHUI class for display images on a center point, with or without a mask.

Test Plan

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.

undefined (1×343 px, 109 KB)

Diff Detail

Repository
rP Phabricator
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

chad retitled this revision from to PHUIImageMask.
chad updated this object.
chad edited the test plan for this revision. (Show Details)
chad added a reviewer: epriestley.

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?)
epriestley edited edge metadata.

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
16

This should use:

celerity_get_resource_uri('/rsrc/image/examples/hero.png');
This revision is now accepted and ready to land.Jun 18 2014, 1:12 PM
chad edited edge metadata.
  • Update per comments

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
107

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

  • Tweak formulas, tesk with actual masks

Just an update for you to play with in SB if you're interested. I still need to build out the image error states.

  • Update selection CSS, add proper exceptions
chad updated this revision to Diff 23087.

Closed by commit rPca801c7ad49a (authored by @chad).

lemme know if I mucked anything up, this feels pretty good in my sandbox. next would be to add it to the comment dialog.