Paths

Table of Contentst

Differential D25897

Added a Mentions tab to the Differential Revision View
Needs ReviewPublic
Actions

Authored by Ekubischta on Wed, Feb 26, 23:06.

Details

Reviewers

None

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T16003: Differential Revision should show "Mentions" tab similar to how Maniphest Tasks do

Summary

This revision adds a Mentions tab to the revision between the History and Commits tabs

The tab functions exactly like the tab you see on a Maniphest task

The reason for this change is that when you have revisions that have lots of activity, or, lots of mentions in other places (including tasks, and other revisions) it is hard to see that list in a concise way

For discussion

Should we extend this to other objects? - And if so, do we need to make this reusable in some way?
- I left the function name as newMentionsTab for consistency with Maniphest for now
Does the tab order matter or is between History and Commits just fine?

Closes T16003

Test Plan

Made a revision, mentioned it, and saw mentions

Diff Detail

Repository

rP Phorge

Branch

T16003-Differential_Revision_Mentions_Tab

Lint

Lint Warnings

Severity	Location	Code	Message
Warning	src/applications/differential/controller/DifferentialRevisionViewController.php:570	XHP25	Spaces Inside Parentheses

Unit

Tests Passed

Build Status

Buildable 1748
Build 1748: arc lint + arc unit

Event Timeline

Ekubischta created this revision.Wed, Feb 26, 23:06

Owners added a reviewer: O1: Blessed Committers.Wed, Feb 26, 23:06

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptWed, Feb 26, 23:06

Ekubischta requested review of this revision.Wed, Feb 26, 23:06

Harbormaster completed remote builds in B1748: Diff 2701.Wed, Feb 26, 23:06

Ekubischta edited the summary of this revision. (Show Details)Wed, Feb 26, 23:07

Ekubischta edited the summary of this revision. (Show Details)Wed, Feb 26, 23:13

Thanks, I can confirm that this works as expected. :)
I agree biggest question is where to put this. In my opinion the UX is already slightly inconsistent (given their are also concepts like "Related tasks" in Pholio or "Referenced Files" in tasks).
In Differential it feels a bit like a stretch to put this into a box called Revision Contents.
In Maniphest the same "Mentions" tab is in the Related Objects box; there is nothing similar in Differential which already has numerous boxes (also Details and Diff Detail but none of those have tabs).

avivey subscribed.Mon, Mar 3, 07:20

avivey added inline comments.

src/applications/differential/controller/DifferentialRevisionViewController.php
62–99	can this be extracted to a utility class and shared with the one from Tasks (and other objects later)?
570	is this lint drunk?

In D25897#24262, @aklapper wrote:

Thanks, I can confirm that this works as expected. :)
I agree biggest question is where to put this. In my opinion the UX is already slightly inconsistent (given their are also concepts like "Related tasks" in Pholio or "Referenced Files" in tasks).
In Differential it feels a bit like a stretch to put this into a box called Revision Contents.
In Maniphest the same "Mentions" tab is in the Related Objects box; there is nothing similar in Differential which already has numerous boxes (also Details and Diff Detail but none of those have tabs).

What if we renamed Revision Contents to something else - Just call it Revision Objects or just Revision? (Or just leave it?)

Note that we even see a Similar tab appear sometimes in this section

Ekubischta added inline comments.Wed, Mar 5, 14:35

src/applications/differential/controller/DifferentialRevisionViewController.php
62–99	Yes - I will work on this I am a bit new to the code base - Do you have a suggestion where it might make the most sense to put this class? Possibly `phorge/src/view/phui/` ?

Revision Contents
Changeset List

Path

Size

Coverage (All)

Coverage (Touched)

Packages

src/

applications/

differential/

controller/

DifferentialRevisionViewController.php

81 lines

O1: Blessed Committers

Diff 2701

View Options

src/applications/differential/controller/DifferentialRevisionViewController.php

Added a Mentions tab to the Differential Revision ViewNeeds ReviewPublicActions