mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-10 08:52:39 +01:00
Conpherence - change "A, B, C..." subtitle to "A: what most recent person said" when we can
Summary: For the price of loading transactions more consistently, we get a better subtitle. We do this in all cases EXCEPT for when we're grabbing handles, because that makes the handles pretty heavy weight and I could even feel the perf hit on my development machine and we don't use subtitle there anyway. We may want to cache the latest message on the conpherence thread object to improve performance here as well as consider falling back to "A, B, C..." more often. Code is written such that no transactions means an automagical fallback. Fixes T7795. (Technically, there's still a note about handle code conversion work on T7795 but we'll get that generally later.) Test Plan: played around with conpherence in both views and things seemed to work nicely. made sure to try the original repro in T7795 and couldn't get that to go either posted a long comment and verified that the CSS / string truncation both make it display nicely. Note that without the CSS the chosen glyph value can be too high to fit nicely at times. Reviewers: chad, epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T7795 Differential Revision: https://secure.phabricator.com/D12347
This commit is contained in:
parent
b467e58358
commit
78bfcc3221
8 changed files with 102 additions and 30 deletions
|
@ -47,7 +47,7 @@ return array(
|
|||
'rsrc/css/application/conpherence/durable-column.css' => 'f0c208ac',
|
||||
'rsrc/css/application/conpherence/menu.css' => 'f389e048',
|
||||
'rsrc/css/application/conpherence/message-pane.css' => 'e44b667b',
|
||||
'rsrc/css/application/conpherence/notification.css' => '04a6e10a',
|
||||
'rsrc/css/application/conpherence/notification.css' => '72178795',
|
||||
'rsrc/css/application/conpherence/update.css' => '1099a660',
|
||||
'rsrc/css/application/conpherence/widget-pane.css' => 'a9082fd0',
|
||||
'rsrc/css/application/contentsource/content-source-view.css' => '4b8b05d4',
|
||||
|
@ -518,7 +518,7 @@ return array(
|
|||
'conpherence-durable-column-view' => 'f0c208ac',
|
||||
'conpherence-menu-css' => 'f389e048',
|
||||
'conpherence-message-pane-css' => 'e44b667b',
|
||||
'conpherence-notification-css' => '04a6e10a',
|
||||
'conpherence-notification-css' => '72178795',
|
||||
'conpherence-thread-manager' => '0a5192c4',
|
||||
'conpherence-update-css' => '1099a660',
|
||||
'conpherence-widget-pane-css' => 'a9082fd0',
|
||||
|
|
|
@ -16,6 +16,7 @@ final class ConpherenceColumnViewController extends
|
|||
$latest_conpherences = id(new ConpherenceThreadQuery())
|
||||
->setViewer($user)
|
||||
->withPHIDs($conpherence_phids)
|
||||
->needTransactions(true)
|
||||
->needParticipantCache(true)
|
||||
->execute();
|
||||
$latest_conpherences = mpull($latest_conpherences, null, 'getPHID');
|
||||
|
|
|
@ -100,6 +100,7 @@ final class ConpherenceListController extends ConpherenceController {
|
|||
} else {
|
||||
$thread = ConpherenceThread::initializeNewThread($user);
|
||||
$thread->attachHandles(array());
|
||||
$thread->attachTransactions(array());
|
||||
$thread->makeEphemeral();
|
||||
$layout->setHeader(
|
||||
$this->buildHeaderPaneContent($thread, array()));
|
||||
|
@ -137,6 +138,7 @@ final class ConpherenceListController extends ConpherenceController {
|
|||
$conpherences = id(new ConpherenceThreadQuery())
|
||||
->setViewer($user)
|
||||
->withPHIDs($conpherence_phids)
|
||||
->needTransactions(true)
|
||||
->needParticipantCache(true)
|
||||
->execute();
|
||||
|
||||
|
|
|
@ -17,6 +17,7 @@ final class ConpherenceNotificationPanelController
|
|||
$conpherences = id(new ConpherenceThreadQuery())
|
||||
->setViewer($user)
|
||||
->withPHIDs(array_keys($participant_data))
|
||||
->needTransactions(true)
|
||||
->needParticipantCache(true)
|
||||
->execute();
|
||||
}
|
||||
|
|
|
@ -380,16 +380,14 @@ final class ConpherenceUpdateController
|
|||
$need_widget_data = false;
|
||||
$need_transactions = false;
|
||||
$need_participant_cache = true;
|
||||
$need_transactions = true;
|
||||
switch ($action) {
|
||||
case ConpherenceUpdateActions::METADATA:
|
||||
$need_transactions = true;
|
||||
break;
|
||||
case ConpherenceUpdateActions::LOAD:
|
||||
$need_transactions = true;
|
||||
break;
|
||||
case ConpherenceUpdateActions::MESSAGE:
|
||||
case ConpherenceUpdateActions::ADD_PERSON:
|
||||
$need_transactions = true;
|
||||
$need_widget_data = true;
|
||||
break;
|
||||
case ConpherenceUpdateActions::REMOVE_PERSON:
|
||||
|
|
|
@ -27,6 +27,7 @@ final class ConpherenceThreadSearchEngine
|
|||
|
||||
public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) {
|
||||
$query = id(new ConpherenceThreadQuery())
|
||||
->needTransactions(true)
|
||||
->needParticipantCache(true);
|
||||
|
||||
$participant_phids = $saved->getParameter('participantPHIDs', array());
|
||||
|
|
|
@ -124,9 +124,12 @@ final class ConpherenceThread extends ConpherenceDAO
|
|||
$this->transactions = $transactions;
|
||||
return $this;
|
||||
}
|
||||
public function getTransactions() {
|
||||
public function getTransactions($assert_attached = true) {
|
||||
return $this->assertAttached($this->transactions);
|
||||
}
|
||||
public function hasAttachedTransactions() {
|
||||
return $this->transactions !== self::ATTACHABLE;
|
||||
}
|
||||
|
||||
public function getTransactionsFrom($begin = 0, $amount = null) {
|
||||
$length = count($this->transactions);
|
||||
|
@ -154,13 +157,32 @@ final class ConpherenceThread extends ConpherenceDAO
|
|||
}
|
||||
|
||||
public function getDisplayData(PhabricatorUser $user) {
|
||||
if ($this->hasAttachedTransactions()) {
|
||||
$transactions = $this->getTransactions();
|
||||
} else {
|
||||
$transactions = array();
|
||||
}
|
||||
$set_title = $this->getTitle();
|
||||
|
||||
if ($set_title) {
|
||||
$title_mode = 'title';
|
||||
} else {
|
||||
$title_mode = 'recent';
|
||||
}
|
||||
|
||||
if ($transactions) {
|
||||
$subtitle_mode = 'message';
|
||||
} else {
|
||||
$subtitle_mode = 'recent';
|
||||
}
|
||||
|
||||
$recent_phids = $this->getRecentParticipantPHIDs();
|
||||
$handles = $this->getHandles();
|
||||
|
||||
// luck has little to do with it really; most recent participant who isn't
|
||||
// the user....
|
||||
// Luck has little to do with it really; most recent participant who
|
||||
// isn't the user....
|
||||
$lucky_phid = null;
|
||||
$lucky_index = null;
|
||||
$recent_title = null;
|
||||
foreach ($recent_phids as $index => $phid) {
|
||||
if ($phid == $user->getPHID()) {
|
||||
continue;
|
||||
|
@ -172,39 +194,82 @@ final class ConpherenceThread extends ConpherenceDAO
|
|||
|
||||
if ($lucky_phid) {
|
||||
$lucky_handle = $handles[$lucky_phid];
|
||||
// this will be just the user talking to themselves. weirdos.
|
||||
} else {
|
||||
// This will be just the user talking to themselves. Weirdo.
|
||||
$lucky_handle = reset($handles);
|
||||
}
|
||||
|
||||
$title = $js_title = $this->getTitle();
|
||||
$img_src = null;
|
||||
if ($lucky_handle) {
|
||||
$img_src = $lucky_handle->getImageURI();
|
||||
}
|
||||
|
||||
$count = 0;
|
||||
$final = false;
|
||||
$subtitle = null;
|
||||
foreach ($recent_phids as $phid) {
|
||||
if ($phid == $user->getPHID()) {
|
||||
continue;
|
||||
if ($title_mode == 'recent' || $subtitle_mode == 'recent') {
|
||||
$count = 0;
|
||||
$final = false;
|
||||
foreach ($recent_phids as $phid) {
|
||||
if ($phid == $user->getPHID()) {
|
||||
continue;
|
||||
}
|
||||
$handle = $handles[$phid];
|
||||
if ($recent_title) {
|
||||
if ($final) {
|
||||
$recent_title .= '...';
|
||||
break;
|
||||
} else {
|
||||
$recent_title .= ', ';
|
||||
}
|
||||
}
|
||||
$recent_title .= $handle->getName();
|
||||
$count++;
|
||||
$final = $count == 3;
|
||||
}
|
||||
$handle = $handles[$phid];
|
||||
if ($subtitle) {
|
||||
if ($final) {
|
||||
$subtitle .= '...';
|
||||
break;
|
||||
} else {
|
||||
$subtitle .= ', ';
|
||||
}
|
||||
|
||||
switch ($title_mode) {
|
||||
case 'recent':
|
||||
$title = $recent_title;
|
||||
$js_title = $recent_title;
|
||||
break;
|
||||
case 'title':
|
||||
$title = $js_title = $this->getTitle();
|
||||
break;
|
||||
}
|
||||
|
||||
$message_title = null;
|
||||
if ($subtitle_mode == 'message') {
|
||||
$message_transaction = null;
|
||||
foreach ($transactions as $transaction) {
|
||||
switch ($transaction->getTransactionType()) {
|
||||
case PhabricatorTransactions::TYPE_COMMENT:
|
||||
$message_transaction = $transaction;
|
||||
break 2;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
}
|
||||
$subtitle .= $handle->getName();
|
||||
$count++;
|
||||
$final = $count == 3;
|
||||
if ($message_transaction) {
|
||||
$message_handle = $handles[$message_transaction->getAuthorPHID()];
|
||||
$message_title = sprintf(
|
||||
'%s: %s',
|
||||
$message_handle->getName(),
|
||||
id(new PhutilUTF8StringTruncator())
|
||||
->setMaximumGlyphs(60)
|
||||
->truncateString(
|
||||
$message_transaction->getComment()->getContent()));
|
||||
}
|
||||
}
|
||||
if (!$title) {
|
||||
$title = $js_title = $subtitle;
|
||||
switch ($subtitle_mode) {
|
||||
case 'recent':
|
||||
$subtitle = $recent_title;
|
||||
break;
|
||||
case 'message':
|
||||
if ($message_title) {
|
||||
$subtitle = $message_title;
|
||||
} else {
|
||||
$subtitle = $recent_title;
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
$user_participation = $this->getParticipantIfExists($user->getPHID());
|
||||
|
|
|
@ -38,7 +38,7 @@
|
|||
font-weight: bold;
|
||||
font-size: 13px;
|
||||
color: {$darkgreytext};
|
||||
width: 280px;
|
||||
width: 314px;
|
||||
text-overflow: ellipsis;
|
||||
white-space: nowrap;
|
||||
overflow: hidden;
|
||||
|
@ -51,6 +51,10 @@
|
|||
font-size: 11px;
|
||||
margin-top: 2px;
|
||||
margin-left: 46px;
|
||||
width: 314px;
|
||||
text-overflow: ellipsis;
|
||||
white-space: nowrap;
|
||||
overflow: hidden;
|
||||
}
|
||||
|
||||
.phabricator-notification .conpherence-menu-item-view
|
||||
|
|
Loading…
Reference in a new issue