From 29d8fc04e50b83fc7cf09f428ef905aa5e54e838 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 28 Mar 2012 10:11:41 -0700 Subject: [PATCH] Improve inline comment previews Summary: - When an inline comment preview corresponds to an inline comment on the page, link to it. Just punt in the tough case where the inline is on some other page. - In "haunted" mode, "z" now toggles through three modes: normal, comment area only, and comment + previews. Test Plan: - Viewed visible and not-visible inline comment previews, clicked "View" links. - Tapped "z" a bunch to toggle haunt modes. Reviewers: btrahan Reviewed By: btrahan CC: aran, epriestley Maniphest Tasks: T517, T214 Differential Revision: https://secure.phabricator.com/D2041 --- .../DifferentialInlineCommentView.php | 34 +++++++++++++------ .../differential/revision-comment.css | 21 +++++++++--- .../differential/behavior-comment-preview.js | 22 +++++++++++- .../differential/behavior-keyboard-nav.js | 13 ++++--- 4 files changed, 69 insertions(+), 21 deletions(-) diff --git a/src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php b/src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php index 604004ac5a..930abcaa83 100644 --- a/src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php +++ b/src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php @@ -136,6 +136,8 @@ final class DifferentialInlineCommentView extends AphrontView { } } + $anchor_name = 'inline-'.$inline->getID(); + if ($this->editable && !$this->preview) { $links[] = javelin_render_tag( 'a', @@ -153,6 +155,16 @@ final class DifferentialInlineCommentView extends AphrontView { 'sigil' => 'differential-inline-delete', ), 'Delete'); + } else if ($this->preview) { + $links[] = javelin_render_tag( + 'a', + array( + 'meta' => array( + 'anchor' => $anchor_name, + ), + 'sigil' => 'differential-inline-preview-jump', + ), + 'Not Visible'); } if ($links) { @@ -178,16 +190,18 @@ final class DifferentialInlineCommentView extends AphrontView { } } - $anchor_name = 'inline-'.$inline->getID(); - - $anchor = phutil_render_tag( - 'a', - array( - 'name' => $anchor_name, - 'id' => $anchor_name, - 'class' => 'differential-inline-comment-anchor', - ), - ''); + if ($this->preview) { + $anchor = null; + } else { + $anchor = phutil_render_tag( + 'a', + array( + 'name' => $anchor_name, + 'id' => $anchor_name, + 'class' => 'differential-inline-comment-anchor', + ), + ''); + } $classes = array( 'differential-inline-comment', diff --git a/webroot/rsrc/css/application/differential/revision-comment.css b/webroot/rsrc/css/application/differential/revision-comment.css index 99f802df03..6fb291101b 100644 --- a/webroot/rsrc/css/application/differential/revision-comment.css +++ b/webroot/rsrc/css/application/differential/revision-comment.css @@ -10,8 +10,14 @@ max-width: 1162px; } -/* Spooky haunted panel which floats on the bottom of the screen. */ -.differential-haunted-panel .differential-add-comment-panel { +/* Spooky haunted panel which floats on the bottom of the screen. + Haunt modes are: + + - Mode 1: Just the comment box. + - Mode 2: Comment box, comment preview, and inline comment previews. +*/ +.differential-haunt-mode-1 .differential-add-comment-panel, +.differential-haunt-mode-2 .differential-add-comment-panel { position: fixed; width: 100%; bottom: 0; @@ -28,15 +34,20 @@ } -.differential-haunted-panel .differential-add-comment-panel h1 { +.differential-haunt-mode-2 .differential-add-comment-panel { + max-height: 75%; +} + +.differential-haunt-mode-1 .differential-add-comment-panel h1, +.differential-haunt-mode-2 .differential-add-comment-panel h1 { display: none; } -.differential-haunted-panel .aphront-panel-preview { +.differential-haunt-mode-1 .aphront-panel-preview { display: none; } -.differential-haunted-panel { +.differential-haunt-mode-1 { padding-bottom: 250px; } diff --git a/webroot/rsrc/js/application/differential/behavior-comment-preview.js b/webroot/rsrc/js/application/differential/behavior-comment-preview.js index 48ffaaa681..104c6e072d 100644 --- a/webroot/rsrc/js/application/differential/behavior-comment-preview.js +++ b/webroot/rsrc/js/application/differential/behavior-comment-preview.js @@ -47,7 +47,27 @@ JX.behavior('differential-feedback-preview', function(config) { function refreshInlinePreview() { new JX.Request(config.inlineuri, function(r) { - JX.DOM.setContent(JX.$(config.inline), JX.$H(r)); + var inline = JX.$(config.inline); + + JX.DOM.setContent(inline, JX.$H(r)); + + // Go through the previews and activate any "View" links where the + // actual comment appears in the document. + + var links = JX.DOM.scry( + inline, + 'a', + 'differential-inline-preview-jump'); + for (var ii = 0; ii < links.length; ii++) { + var data = JX.Stratcom.getData(links[ii]); + try { + JX.$(data.anchor); + links[ii].href = '#' + data.anchor; + JX.DOM.setContent(links[ii], 'View'); + } catch (ignored) { + // This inline comment isn't visible, e.g. on some other diff. + } + } }) .setTimeout(5000) .send(); diff --git a/webroot/rsrc/js/application/differential/behavior-keyboard-nav.js b/webroot/rsrc/js/application/differential/behavior-keyboard-nav.js index 2944095d47..b465efc5f3 100644 --- a/webroot/rsrc/js/application/differential/behavior-keyboard-nav.js +++ b/webroot/rsrc/js/application/differential/behavior-keyboard-nav.js @@ -150,11 +150,14 @@ JX.behavior('differential-keyboard-navigation', function(config) { changesets = null; }); - var is_haunted = false; + var haunt_mode = 0; function haunt() { - is_haunted = !is_haunted; - var haunt = JX.$(config.haunt) - JX.DOM.alterClass(haunt, 'differential-haunted-panel', is_haunted); + haunt_mode = (haunt_mode + 1) % 3; + + var el = JX.$(config.haunt); + for (var ii = 1; ii <= 2; ii++) { + JX.DOM.alterClass(el, 'differential-haunt-mode-'+ii, (haunt_mode == ii)); + } } new JX.KeyboardShortcut('j', 'Jump to next change.') @@ -221,7 +224,7 @@ JX.behavior('differential-keyboard-navigation', function(config) { .register(); if (config.haunt) { - new JX.KeyboardShortcut('z', 'Haunt / unhaunt comment panel.') + new JX.KeyboardShortcut('z', 'Cycle comment panel haunting modes.') .setHandler(haunt) .register(); }