mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-10 00:42:41 +01:00
Improve display behavior of commit messages in Diffusion
Summary: See T372. Always render commit messages on one display line, so the table doesn't jump around as they AJAX in on browse views. The goal here is to have the cell choose a size naturally and for its content to render with "overflow: hidden" if the natural size isn't large enough to contain the content. "white-space: pre" or "white-space: nowrap" would prevent wrapping but potentially make the table exceed the display width when a better behavior is to hide some of the commit message. Also use utf8-aware shortening, now that we have a function for it. Casting a wide net in case anyone has a better way to do the CSS here. It's kind of nasty that we have to use so many DOM nodes. Test Plan: - Resized window while viewing browse and history views in Safari, Chrome and Firefox. Table exhibited described behavior. - Verified summaries render sensibly and are properly truncated to 100 characters. Reviewed By: aran Reviewers: aran, jungejason, tuomaspelkonen, tomo, mroch, cpojer CC: aran, epriestley Differential Revision: 750
This commit is contained in:
parent
1048669158
commit
35d03d36c7
6 changed files with 44 additions and 4 deletions
|
@ -57,7 +57,8 @@ final class DiffusionBrowseTableView extends DiffusionView {
|
|||
} else {
|
||||
$author = phutil_escape_html($data->getAuthorName());
|
||||
}
|
||||
$details = phutil_escape_html($data->getSummary());
|
||||
$details = AphrontTableView::renderSingleDisplayLine(
|
||||
phutil_escape_html($data->getSummary()));
|
||||
} else {
|
||||
$author = '';
|
||||
$details = '';
|
||||
|
|
|
@ -77,7 +77,8 @@ final class DiffusionHistoryTableView extends DiffusionView {
|
|||
$date,
|
||||
$time,
|
||||
$author,
|
||||
phutil_escape_html($history->getSummary()),
|
||||
AphrontTableView::renderSingleDisplayLine(
|
||||
phutil_escape_html($history->getSummary())),
|
||||
// TODO: etc etc
|
||||
);
|
||||
}
|
||||
|
@ -101,7 +102,7 @@ final class DiffusionHistoryTableView extends DiffusionView {
|
|||
'',
|
||||
'right',
|
||||
'',
|
||||
'wide wrap',
|
||||
'wide',
|
||||
));
|
||||
return $view->render();
|
||||
}
|
||||
|
|
|
@ -38,7 +38,8 @@ class PhabricatorRepositoryCommitData extends PhabricatorRepositoryDAO {
|
|||
$message = $this->getCommitMessage();
|
||||
$lines = explode("\n", $message);
|
||||
$summary = head($lines);
|
||||
$summary = substr($summary, 0, self::SUMMARY_MAX_LENGTH);
|
||||
|
||||
$summary = phutil_utf8_shorten($summary, self::SUMMARY_MAX_LENGTH);
|
||||
|
||||
return $summary;
|
||||
}
|
||||
|
|
|
@ -154,5 +154,29 @@ class AphrontTableView extends AphrontView {
|
|||
$table[] = '</table>';
|
||||
return implode('', $table);
|
||||
}
|
||||
|
||||
public static function renderSingleDisplayLine($line) {
|
||||
|
||||
// TODO: Is there a cleaner way to do this? We use a relative div with
|
||||
// overflow hidden to provide the bounds, and an absolute span with
|
||||
// white-space: pre to prevent wrapping. We need to append a character
|
||||
// ( -- nonbreaking space) afterward to give the bounds div height
|
||||
// (alternatively, we could hard-code the line height). This is gross but
|
||||
// it's not clear that there's a better appraoch.
|
||||
|
||||
return phutil_render_tag(
|
||||
'div',
|
||||
array(
|
||||
'class' => 'single-display-line-bounds',
|
||||
),
|
||||
phutil_render_tag(
|
||||
'span',
|
||||
array(
|
||||
'class' => 'single-display-line-content',
|
||||
),
|
||||
$line).' ');
|
||||
}
|
||||
|
||||
|
||||
}
|
||||
|
||||
|
|
|
@ -9,6 +9,7 @@
|
|||
phutil_require_module('phabricator', 'infrastructure/celerity/api');
|
||||
phutil_require_module('phabricator', 'view/base');
|
||||
|
||||
phutil_require_module('phutil', 'markup');
|
||||
phutil_require_module('phutil', 'utils');
|
||||
|
||||
|
||||
|
|
|
@ -74,6 +74,17 @@
|
|||
white-space: normal;
|
||||
}
|
||||
|
||||
div.single-display-line-bounds {
|
||||
width: 100%;
|
||||
position: relative;
|
||||
overflow: hidden;
|
||||
}
|
||||
|
||||
span.single-display-line-content {
|
||||
white-space: pre;
|
||||
position: absolute;
|
||||
}
|
||||
|
||||
.aphront-table-view tr.highlighted {
|
||||
background: #ffff99;
|
||||
}
|
||||
|
@ -94,3 +105,4 @@
|
|||
max-width: 64px;
|
||||
max-height: 64px;
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in a new issue