From b712905dc1b16a06cc5b3f047390fbe24741f937 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 5 Aug 2013 10:47:26 -0700 Subject: [PATCH] Add a "document" style to PHUIRemarkupPreviewPanel and use it in Legalpad and Phriction Summary: Ref T3671. Depends on D6674. Continues work in D6673, D6674 and extends it into Legalpad and Phriction. Then deletes a bunch of dead code. Test Plan: Edited documents in Legalpad and Phriction, verified I got reasonable looking previews. Reviewers: btrahan, Firehed Reviewed By: btrahan CC: aran, chad Maniphest Tasks: T3671 Differential Revision: https://secure.phabricator.com/D6675 --- src/__celerity_resource_map__.php | 30 +-------------- src/__phutil_library_map__.php | 2 - .../PhabricatorApplicationLegalpad.php | 2 +- .../LegalpadDocumentEditController.php | 29 ++++----------- .../LegalpadDocumentPreviewController.php | 26 ------------- .../PhabricatorApplicationPhriction.php | 2 +- .../controller/PhrictionEditController.php | 25 +++---------- .../ponder/storage/PonderAnswer.php | 2 +- .../ponder/storage/PonderComment.php | 2 +- .../ponder/storage/PonderQuestion.php | 2 +- .../markup/PhabricatorMarkupEngine.php | 14 ------- .../markup/PhabricatorMarkupOneOff.php | 8 +++- .../PhabricatorMarkupPreviewController.php | 4 +- src/view/phui/PHUIRemarkupPreviewPanel.php | 37 ++++++++++++++++++- .../phriction/phriction-document-css.css | 6 --- .../rsrc/css/phui/phui-remarkup-preview.css | 21 +++++++++++ .../legalpad/legalpad-document-preview.js | 33 ----------------- .../phriction/phriction-document-preview.js | 29 --------------- 18 files changed, 86 insertions(+), 188 deletions(-) delete mode 100644 src/applications/legalpad/controller/LegalpadDocumentPreviewController.php delete mode 100644 webroot/rsrc/js/application/legalpad/legalpad-document-preview.js delete mode 100644 webroot/rsrc/js/application/phriction/phriction-document-preview.js diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 8fa5e928de..5528c06f04 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -1765,19 +1765,6 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/core/behavior-konami.js', ), - 'javelin-behavior-legalpad-document-preview' => - array( - 'uri' => '/res/d0ce5a8c/rsrc/js/application/legalpad/legalpad-document-preview.js', - 'type' => 'js', - 'requires' => - array( - 0 => 'javelin-behavior', - 1 => 'javelin-dom', - 2 => 'javelin-util', - 3 => 'phabricator-shaped-request', - ), - 'disk' => '/rsrc/js/application/legalpad/legalpad-document-preview.js', - ), 'javelin-behavior-lightbox-attachments' => array( 'uri' => '/res/72b4d3a8/rsrc/js/core/behavior-lightbox-attachments.js', @@ -2255,19 +2242,6 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/application/pholio/behavior-pholio-mock-view.js', ), - 'javelin-behavior-phriction-document-preview' => - array( - 'uri' => '/res/e2fe02de/rsrc/js/application/phriction/phriction-document-preview.js', - 'type' => 'js', - 'requires' => - array( - 0 => 'javelin-behavior', - 1 => 'javelin-dom', - 2 => 'javelin-util', - 3 => 'phabricator-shaped-request', - ), - 'disk' => '/rsrc/js/application/phriction/phriction-document-preview.js', - ), 'javelin-behavior-ponder-votebox' => array( 'uri' => '/res/c28daa12/rsrc/js/application/ponder/behavior-votebox.js', @@ -3781,7 +3755,7 @@ celerity_register_resource_map(array( ), 'phriction-document-css' => array( - 'uri' => '/res/97a9ef40/rsrc/css/application/phriction/phriction-document-css.css', + 'uri' => '/res/754f6b37/rsrc/css/application/phriction/phriction-document-css.css', 'type' => 'css', 'requires' => array( @@ -3853,7 +3827,7 @@ celerity_register_resource_map(array( ), 'phui-remarkup-preview-css' => array( - 'uri' => '/res/80d54c8c/rsrc/css/phui/phui-remarkup-preview.css', + 'uri' => '/res/6c886e63/rsrc/css/phui/phui-remarkup-preview.css', 'type' => 'css', 'requires' => array( diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index ebf0ac40fb..d6ea568921 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -647,7 +647,6 @@ phutil_register_library_map(array( 'LegalpadDocumentEditController' => 'applications/legalpad/controller/LegalpadDocumentEditController.php', 'LegalpadDocumentEditor' => 'applications/legalpad/editor/LegalpadDocumentEditor.php', 'LegalpadDocumentListController' => 'applications/legalpad/controller/LegalpadDocumentListController.php', - 'LegalpadDocumentPreviewController' => 'applications/legalpad/controller/LegalpadDocumentPreviewController.php', 'LegalpadDocumentQuery' => 'applications/legalpad/query/LegalpadDocumentQuery.php', 'LegalpadDocumentSearchEngine' => 'applications/legalpad/query/LegalpadDocumentSearchEngine.php', 'LegalpadDocumentSignController' => 'applications/legalpad/controller/LegalpadDocumentSignController.php', @@ -2657,7 +2656,6 @@ phutil_register_library_map(array( 0 => 'LegalpadController', 1 => 'PhabricatorApplicationSearchResultsControllerInterface', ), - 'LegalpadDocumentPreviewController' => 'LegalpadController', 'LegalpadDocumentQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'LegalpadDocumentSearchEngine' => 'PhabricatorApplicationSearchEngine', 'LegalpadDocumentSignController' => 'LegalpadController', diff --git a/src/applications/legalpad/application/PhabricatorApplicationLegalpad.php b/src/applications/legalpad/application/PhabricatorApplicationLegalpad.php index 913d5d2d26..6071d2b62b 100644 --- a/src/applications/legalpad/application/PhabricatorApplicationLegalpad.php +++ b/src/applications/legalpad/application/PhabricatorApplicationLegalpad.php @@ -49,7 +49,7 @@ final class PhabricatorApplicationLegalpad extends PhabricatorApplication { 'comment/(?P\d+)/' => 'LegalpadDocumentCommentController', 'view/(?P\d+)/' => 'LegalpadDocumentViewController', 'document/' => array( - 'preview/' => 'LegalpadDocumentPreviewController'), + 'preview/' => 'PhabricatorMarkupPreviewController'), )); } diff --git a/src/applications/legalpad/controller/LegalpadDocumentEditController.php b/src/applications/legalpad/controller/LegalpadDocumentEditController.php index a3d8e1b5f9..11f04fa3c6 100644 --- a/src/applications/legalpad/controller/LegalpadDocumentEditController.php +++ b/src/applications/legalpad/controller/LegalpadDocumentEditController.php @@ -172,34 +172,19 @@ final class LegalpadDocumentEditController extends LegalpadController { $crumbs->addCrumb( id(new PhabricatorCrumbView())->setName($short)); - $preview_header = id(new PhabricatorHeaderView()) - ->setHeader(pht('Document Preview')); - $preview_view = phutil_tag( - 'div', - array( - 'id' => 'document-preview'), - phutil_tag( - 'div', - array( - 'class' => 'aphront-panel-preview-loading-text'), - pht('Loading preview...'))); - $preview_panel = id(new PHUIDocumentView()) - ->appendChild($preview_header) - ->appendChild($preview_view); - Javelin::initBehavior( - 'legalpad-document-preview', - array( - 'preview' => 'document-preview', - 'title' => 'document-title', - 'text' => 'document-text', - 'uri' => $this->getApplicationURI('document/preview/'))); + + $preview = id(new PHUIRemarkupPreviewPanel()) + ->setHeader(pht('Document Preview')) + ->setPreviewURI($this->getApplicationURI('document/preview/')) + ->setControlID('document-text') + ->setSkin('document'); return $this->buildApplicationPage( array( $crumbs, $error_view, $form, - $preview_panel + $preview ), array( 'title' => $title, diff --git a/src/applications/legalpad/controller/LegalpadDocumentPreviewController.php b/src/applications/legalpad/controller/LegalpadDocumentPreviewController.php deleted file mode 100644 index 5aac38e9f9..0000000000 --- a/src/applications/legalpad/controller/LegalpadDocumentPreviewController.php +++ /dev/null @@ -1,26 +0,0 @@ -getRequest(); - $user = $request->getUser(); - $text = $request->getStr('text'); - - $body = id(new LegalpadDocumentBody()) - ->setText($text); - - $content = PhabricatorMarkupEngine::renderOneObject( - $body, - LegalpadDocumentBody::MARKUP_FIELD_TEXT, - $user); - - $content = hsprintf('
%s
', $content); - - return id(new AphrontAjaxResponse())->setContent($content); - } -} diff --git a/src/applications/phriction/application/PhabricatorApplicationPhriction.php b/src/applications/phriction/application/PhabricatorApplicationPhriction.php index 508dc772cc..d0573e872b 100644 --- a/src/applications/phriction/application/PhabricatorApplicationPhriction.php +++ b/src/applications/phriction/application/PhabricatorApplicationPhriction.php @@ -52,7 +52,7 @@ final class PhabricatorApplicationPhriction extends PhabricatorApplication { 'new/' => 'PhrictionNewController', 'move/(?:(?P[1-9]\d*)/)?' => 'PhrictionMoveController', - 'preview/' => 'PhrictionDocumentPreviewController', + 'preview/' => 'PhabricatorMarkupPreviewController', 'diff/(?P[1-9]\d*)/' => 'PhrictionDiffController', ), ); diff --git a/src/applications/phriction/controller/PhrictionEditController.php b/src/applications/phriction/controller/PhrictionEditController.php index 56ac847c1a..5437cfec98 100644 --- a/src/applications/phriction/controller/PhrictionEditController.php +++ b/src/applications/phriction/controller/PhrictionEditController.php @@ -229,24 +229,11 @@ final class PhrictionEditController $header = id(new PhabricatorHeaderView()) ->setHeader($panel_header); - $preview_content = hsprintf( - '
%s
-
-
%s
-
', - pht('Document Preview'), - pht('Loading preview...')); - - $preview_panel = id(new PHUIDocumentView()) - ->appendChild($preview_content); - - Javelin::initBehavior( - 'phriction-document-preview', - array( - 'preview' => 'document-preview', - 'textarea' => 'document-textarea', - 'uri' => '/phriction/preview/?draftkey='.$draft_key, - )); + $preview = id(new PHUIRemarkupPreviewPanel()) + ->setHeader(pht('Document Preview')) + ->setPreviewURI('/phriction/preview/') + ->setControlID('document-textarea') + ->setSkin('document'); $crumbs = $this->buildApplicationCrumbs(); if ($document->getID()) { @@ -269,7 +256,7 @@ final class PhrictionEditController $draft_note, $error_view, $form, - $preview_panel, + $preview, ), array( 'title' => pht('Edit Document'), diff --git a/src/applications/ponder/storage/PonderAnswer.php b/src/applications/ponder/storage/PonderAnswer.php index e9d6234345..c96e71b701 100644 --- a/src/applications/ponder/storage/PonderAnswer.php +++ b/src/applications/ponder/storage/PonderAnswer.php @@ -100,7 +100,7 @@ final class PonderAnswer extends PonderDAO } public function newMarkupEngine($field) { - return PhabricatorMarkupEngine::newPonderMarkupEngine(); + return PhabricatorMarkupEngine::getEngine(); } public function didMarkupText( diff --git a/src/applications/ponder/storage/PonderComment.php b/src/applications/ponder/storage/PonderComment.php index b26568f7d4..b1abf2fa9d 100644 --- a/src/applications/ponder/storage/PonderComment.php +++ b/src/applications/ponder/storage/PonderComment.php @@ -20,7 +20,7 @@ final class PonderComment extends PonderDAO } public function newMarkupEngine($field) { - return PhabricatorMarkupEngine::newPonderMarkupEngine(); + return PhabricatorMarkupEngine::getEngine(); } public function didMarkupText( diff --git a/src/applications/ponder/storage/PonderQuestion.php b/src/applications/ponder/storage/PonderQuestion.php index 35a7dc27a3..d01d70eb4c 100644 --- a/src/applications/ponder/storage/PonderQuestion.php +++ b/src/applications/ponder/storage/PonderQuestion.php @@ -146,7 +146,7 @@ final class PonderQuestion extends PonderDAO } public function newMarkupEngine($field) { - return PhabricatorMarkupEngine::newPonderMarkupEngine(); + return PhabricatorMarkupEngine::getEngine(); } public function didMarkupText( diff --git a/src/infrastructure/markup/PhabricatorMarkupEngine.php b/src/infrastructure/markup/PhabricatorMarkupEngine.php index ee407a68c8..bc25b371e7 100644 --- a/src/infrastructure/markup/PhabricatorMarkupEngine.php +++ b/src/infrastructure/markup/PhabricatorMarkupEngine.php @@ -345,20 +345,6 @@ final class PhabricatorMarkupEngine { )); } - - /** - * @task engine - */ - public static function newSlowvoteMarkupEngine() { - return self::newMarkupEngine(array( - )); - } - - - public static function newPonderMarkupEngine(array $options = array()) { - return self::newMarkupEngine($options); - } - /** * @task engine */ diff --git a/src/infrastructure/markup/PhabricatorMarkupOneOff.php b/src/infrastructure/markup/PhabricatorMarkupOneOff.php index 4bdab12c2a..f0f213fc12 100644 --- a/src/infrastructure/markup/PhabricatorMarkupOneOff.php +++ b/src/infrastructure/markup/PhabricatorMarkupOneOff.php @@ -15,6 +15,12 @@ final class PhabricatorMarkupOneOff implements PhabricatorMarkupInterface { private $content; + private $preserveLinebreaks; + + public function setPreserveLinebreaks($preserve_linebreaks) { + $this->preserveLinebreaks = $preserve_linebreaks; + return $this; + } public function setContent($content) { $this->content = $content; @@ -32,7 +38,7 @@ final class PhabricatorMarkupOneOff implements PhabricatorMarkupInterface { public function newMarkupEngine($field) { return PhabricatorMarkupEngine::newMarkupEngine( array( - 'preserve-linebreaks' => false, + 'preserve-linebreaks' => $this->preserveLinebreaks, )); } diff --git a/src/infrastructure/markup/PhabricatorMarkupPreviewController.php b/src/infrastructure/markup/PhabricatorMarkupPreviewController.php index dc0c264fba..5417df09ab 100644 --- a/src/infrastructure/markup/PhabricatorMarkupPreviewController.php +++ b/src/infrastructure/markup/PhabricatorMarkupPreviewController.php @@ -10,7 +10,9 @@ final class PhabricatorMarkupPreviewController $text = $request->getStr('text'); $output = PhabricatorMarkupEngine::renderOneObject( - id(new PhabricatorMarkupOneOff())->setContent($text), + id(new PhabricatorMarkupOneOff()) + ->setPreserveLinebreaks(true) + ->setContent($text), 'default', $viewer); diff --git a/src/view/phui/PHUIRemarkupPreviewPanel.php b/src/view/phui/PHUIRemarkupPreviewPanel.php index d0efb01057..fa528a3c1a 100644 --- a/src/view/phui/PHUIRemarkupPreviewPanel.php +++ b/src/view/phui/PHUIRemarkupPreviewPanel.php @@ -9,6 +9,7 @@ final class PHUIRemarkupPreviewPanel extends AphrontTagView { private $loadingText; private $controlID; private $previewURI; + private $skin = 'default'; protected function canAppendChild() { return false; @@ -34,13 +35,35 @@ final class PHUIRemarkupPreviewPanel extends AphrontTagView { return $this; } + public function setSkin($skin) { + static $skins = array( + 'default' => true, + 'document' => true, + ); + + if (empty($skins[$skin])) { + $valid = implode(', ', array_keys($skins)); + throw new Exception("Invalid skin '{$skin}'. Valid skins are: {$valid}."); + } + + $this->skin = $skin; + return $this; + } + public function getTagName() { return 'div'; } public function getTagAttributes() { + $classes = array(); + $classes[] = 'phui-remarkup-preview'; + + if ($this->skin) { + $classes[] = 'phui-remarkup-preview-skin-'.$this->skin; + } + return array( - 'class' => 'phui-remarkup-preview', + 'class' => $classes, ); } @@ -88,7 +111,17 @@ final class PHUIRemarkupPreviewPanel extends AphrontTagView { ), $loading); - return array($header, $preview); + $content = array($header, $preview); + + switch ($this->skin) { + case 'document': + $content = id(new PHUIDocumentView())->appendChild($content); + break; + default: + break; + } + + return $content; } } diff --git a/webroot/rsrc/css/application/phriction/phriction-document-css.css b/webroot/rsrc/css/application/phriction/phriction-document-css.css index 8d12cad149..f04a1e147a 100644 --- a/webroot/rsrc/css/application/phriction/phriction-document-css.css +++ b/webroot/rsrc/css/application/phriction/phriction-document-css.css @@ -40,12 +40,6 @@ margin-bottom: 15px; } -.phriction-document-preview-header { - color: #666666; - margin-bottom: 1em; - font-size: 11px; -} - .phriction-document-history-diff { padding: 0 2em 2em; } diff --git a/webroot/rsrc/css/phui/phui-remarkup-preview.css b/webroot/rsrc/css/phui/phui-remarkup-preview.css index cfc59b6e83..89cdcb6582 100644 --- a/webroot/rsrc/css/phui/phui-remarkup-preview.css +++ b/webroot/rsrc/css/phui/phui-remarkup-preview.css @@ -14,6 +14,16 @@ padding: 12px; } +.phui-remarkup-preview-skin-document { + background: transparent; + border: none; +} + +.phui-remarkup-preview-skin-document .phui-preview-header { + padding: 8px; + background: #f3f3f3; +} + .device-phone .aphront-panel-preview { display: none; } @@ -21,3 +31,14 @@ .phui-preview-loading-text { color: #666666; } + +/** + * TODO: Classes implementing PhabricatorMarkupInterface are of differing + * mindsets about whether output should be wrapped in a `phabricator-remarkup` + *
or not. It should probably move to the Engine in all cases, but + * until we do that get rid of the extra spacing generated by the inner div. + */ +.phui-remarkup-preview .phabricator-remarkup .phabricator-remarkup { + padding: 0; + margin: 0; +} diff --git a/webroot/rsrc/js/application/legalpad/legalpad-document-preview.js b/webroot/rsrc/js/application/legalpad/legalpad-document-preview.js deleted file mode 100644 index 9826bdc593..0000000000 --- a/webroot/rsrc/js/application/legalpad/legalpad-document-preview.js +++ /dev/null @@ -1,33 +0,0 @@ -/** - * @provides javelin-behavior-legalpad-document-preview - * @requires javelin-behavior - * javelin-dom - * javelin-util - * phabricator-shaped-request - */ - -JX.behavior('legalpad-document-preview', function(config) { - - var preview = JX.$(config.preview); - var title = JX.$(config.title); - var text = JX.$(config.text); - - var callback = function(r) { - JX.DOM.setContent(JX.$(config.preview), JX.$H(r)); - }; - - var getdata = function() { - return { - title : title.value, - text : text.value - }; - }; - - var request = new JX.PhabricatorShapedRequest(config.uri, callback, getdata); - var trigger = JX.bind(request, request.trigger); - - JX.DOM.listen(title, 'keydown', null, trigger); - JX.DOM.listen(text, 'keydown', null, trigger); - request.start(); - -}); diff --git a/webroot/rsrc/js/application/phriction/phriction-document-preview.js b/webroot/rsrc/js/application/phriction/phriction-document-preview.js deleted file mode 100644 index c806482d4e..0000000000 --- a/webroot/rsrc/js/application/phriction/phriction-document-preview.js +++ /dev/null @@ -1,29 +0,0 @@ -/** - * @provides javelin-behavior-phriction-document-preview - * @requires javelin-behavior - * javelin-dom - * javelin-util - * phabricator-shaped-request - */ - -JX.behavior('phriction-document-preview', function(config) { - - var preview = JX.$(config.preview); - var textarea = JX.$(config.textarea); - - var callback = function(r) { - JX.DOM.setContent(JX.$(config.preview), JX.$H(r)); - }; - - var getdata = function() { - return { - document : textarea.value - }; - }; - - var request = new JX.PhabricatorShapedRequest(config.uri, callback, getdata); - var trigger = JX.bind(request, request.trigger); - - JX.DOM.listen(textarea, 'keydown', null, trigger); - request.start(); -});