Page MenuHomePhabricator

Migrate DiffusionBlameController to use repo identities
ClosedPublic

Authored by amckinley on Aug 14 2018, 11:46 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Mar 23, 3:59 PM
Unknown Object (File)
Wed, Mar 13, 4:50 AM
Unknown Object (File)
Feb 21 2024, 5:33 AM
Unknown Object (File)
Feb 9 2024, 8:12 AM
Unknown Object (File)
Jan 23 2024, 11:05 AM
Unknown Object (File)
Jan 1 2024, 1:01 AM
Unknown Object (File)
Dec 26 2023, 9:17 AM
Unknown Object (File)
Dec 24 2023, 3:20 AM
Subscribers

Details

Summary

Now on the blame page, identities get avatar.png and there are little tooltips that show a few characters of the committer identity string.

Also add a default icon for repo identities.

Test Plan

Loaded some blame pages for files touched by users with and without repo identities attached.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

src/applications/diffusion/controller/DiffusionBlameController.php
130

Does something like this look OK?

if (strlen($author_src)) {
  $author_style = ...;
  $author_icon = null;
} else {
  $author_style = null;
  $author_icon = id(new PHUIIconView())->setIcon('fa-something-something');
}

Then $author_link = javelin_tag(..., ..., $author_icon) down below. Basically, plug in a little identity icon instead of an actual background image if we have an Identity instead of a real user. This might make the CSS go craaaaaazy, though.

Alternatively, we could change if (!$author_phid) to if (!$author_phid || !$image_uri), which would "fix" this without a regression, but it would be nice to actually render something useful here instead of just giving up.

Failing this, you can use PhabricatorFile::loadBuiltin(...) and drop some static image file in resources/builtin/ to get a real image file, something like:

$identity_file = PhabricatorFile::loadBuiltin($viewer, 'identity-default-profile.png');
$identity_src = $identity_file->getViewURI();
src/applications/diffusion/controller/DiffusionBlameController.php
120–121

We could probably drop this bit of code now -- it should be exceptionally rare now, since we'll almost always have an Identity even if we don't have a real user. (Before, it was a reasonable fallback if you committed with a weird name but we found a revision and could kind of guess things.)

Use avatar.png as a default image for identities.

amckinley marked an inline comment as done.

Fix a problem with truncated tooltips by setting size to auto.

src/applications/diffusion/controller/DiffusionBlameController.php
130

I was leaning towards a change in PhabricatorRepositoryIdentityPHIDType to solve this everywhere we might want to render identity objects. Let me know if this is a Bad Idea for some reason.

Ah, yeah, this is even simpler.

This revision is now accepted and ready to land.Sep 4 2018, 3:32 PM
This revision was automatically updated to reflect the committed changes.