Modernize the display of comments on a change

We now display comments for a change more like Gmail might, with
short summaries of the body when the message is collapsed and a
full message expanded out when clicked.

User names are shown bold, but the email address is hidden under
a tooltip if you need more information.  Dates are shown in the
short format used on dashboards, but again a tooltip shows the
more complete timestamp information when necessary.

Change-Id: Ic2f162ad49f4bddfb66d7bf69f2348091ec5e3b7
Signed-off-by: Shawn O. Pearce <sop@google.com>

src/main/java/com/google/gerrit/client/changes/ChangeConstants.java

@@ -66,7 +66,7 @@ public interface ChangeConstants extends Constants {
   String changeScreenDependencies();
   String changeScreenDependsOn();
   String changeScreenNeededBy();
-  String changeScreenMessages();
+  String changeScreenComments();
 
   String approvalTableReviewer();
   String approvalTableAddReviewer();

src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties

@@ -43,7 +43,7 @@ patchTableOpen = Open file
 changeScreenDependencies =  Dependencies
 changeScreenDependsOn = Depends On
 changeScreenNeededBy = Needed By
-changeScreenMessages = Messages
+changeScreenComments = Comments
 
 approvalTableReviewer = Reviewer
 approvalTableAddReviewer = Add Reviewer

src/main/java/com/google/gerrit/client/changes/ChangeScreen.java

@@ -14,18 +14,20 @@
 
 package com.google.gerrit.client.changes;
 
-import com.google.gerrit.client.FormatUtil;
 import com.google.gerrit.client.Gerrit;
 import com.google.gerrit.client.Link;
+import com.google.gerrit.client.data.AccountInfo;
 import com.google.gerrit.client.data.AccountInfoCache;
 import com.google.gerrit.client.data.ChangeDetail;
 import com.google.gerrit.client.data.ChangeInfo;
 import com.google.gerrit.client.data.GitwebLink;
+import com.google.gerrit.client.reviewdb.Account;
 import com.google.gerrit.client.reviewdb.Change;
 import com.google.gerrit.client.reviewdb.ChangeMessage;
 import com.google.gerrit.client.reviewdb.PatchSet;
 import com.google.gerrit.client.rpc.GerritCallback;
 import com.google.gerrit.client.rpc.ScreenLoadCallback;
+import com.google.gerrit.client.ui.CommentPanel;
 import com.google.gerrit.client.ui.ComplexDisclosurePanel;
 import com.google.gerrit.client.ui.ExpandAllCommand;
 import com.google.gerrit.client.ui.LinkMenuBar;
@@ -42,8 +44,8 @@ import com.google.gwt.user.client.ui.DisclosurePanel;
 import com.google.gwt.user.client.ui.FlowPanel;
 import com.google.gwt.user.client.ui.Image;
 import com.google.gwt.user.client.ui.InlineLabel;
+import com.google.gwt.user.client.ui.Label;
 import com.google.gwt.user.client.ui.Panel;
-import com.google.gwt.user.client.ui.Widget;
 import com.google.gwtexpui.globalkey.client.GlobalKey;
 import com.google.gwtexpui.globalkey.client.KeyCommand;
 import com.google.gwtexpui.globalkey.client.KeyCommandSet;
@@ -69,8 +71,7 @@ public class ChangeScreen extends Screen {
 
   private FlowPanel patchSetPanels;
 
-  private DisclosurePanel messagesPanel;
-  private Panel messagesContent;
+  private Panel comments;
 
   private KeyCommandSet keysNavigation;
   private KeyCommandSet keysAction;
@@ -187,11 +188,9 @@ public class ChangeScreen extends Screen {
     patchSetPanels = new FlowPanel();
     add(patchSetPanels);
 
-    messagesContent = new FlowPanel();
-    messagesContent.setStyleName("gerrit-ChangeMessages");
-    messagesPanel = new DisclosurePanel(Util.C.changeScreenMessages());
-    messagesPanel.setContent(messagesContent);
-    add(messagesPanel);
+    comments = new FlowPanel();
+    comments.setStyleName("gerrit-ChangeComments");
+    add(comments);
   }
 
   private void displayTitle(final String subject) {
@@ -226,7 +225,7 @@ public class ChangeScreen extends Screen {
         .getApprovals());
 
     addPatchSets(detail);
-    addMessages(detail);
+    addComments(detail);
 
     // If any dependency change is still open, show our dependency list.
     //
@@ -283,13 +282,17 @@ public class ChangeScreen extends Screen {
     currentPatchSet = currps.getId();
   }
 
-  private void addMessages(final ChangeDetail detail) {
-    messagesContent.clear();
+  private void addComments(final ChangeDetail detail) {
+    comments.clear();
+
+    final Label hdr = new Label(Util.C.changeScreenComments());
+    hdr.setStyleName("gerrit-BlockHeader");
+    comments.add(hdr);
 
     final AccountInfoCache accts = detail.getAccounts();
     final List<ChangeMessage> msgList = detail.getMessages();
     if (msgList.size() > 1) {
-      messagesContent.add(messagesMenuBar());
+      comments.add(messagesMenuBar());
     }
 
     final long AGE = 7 * 24 * 60 * 60 * 1000L;
@@ -297,46 +300,47 @@ public class ChangeScreen extends Screen {
 
     for (int i = 0; i < msgList.size(); i++) {
       final ChangeMessage msg = msgList.get(i);
-      final MessagePanel mp = new MessagePanel(msg);
-      final String panelHeader;
-      final ComplexDisclosurePanel panel;
 
+      final AccountInfo author;
       if (msg.getAuthor() != null) {
-        panelHeader = FormatUtil.nameEmail(accts.get(msg.getAuthor()));
+        author = accts.get(msg.getAuthor());
       } else {
-        panelHeader = Util.C.messageNoAuthor();
+        final Account gerrit = new Account(null);
+        gerrit.setFullName(Util.C.messageNoAuthor());
+        author = new AccountInfo(gerrit);
       }
 
+      boolean isRecent;
       if (i == msgList.size() - 1) {
-        mp.isRecent = true;
+        isRecent = true;
       } else {
         // TODO Instead of opening messages by strict age, do it by "unread"?
-        mp.isRecent = msg.getWrittenOn().after(aged);
+        isRecent = msg.getWrittenOn().after(aged);
       }
 
-      panel = new ComplexDisclosurePanel(panelHeader, mp.isRecent);
-      panel.getHeader().add(
-          new InlineLabel(Util.M.messageWrittenOn(FormatUtil.mediumFormat(msg
-              .getWrittenOn()))));
-      panel.setContent(mp);
-      messagesContent.add(panel);
+      final CommentPanel cp =
+          new CommentPanel(author, msg.getWrittenOn(), msg.getMessage());
+      cp.setRecent(isRecent);
+      if (i == msgList.size() - 1) {
+        cp.addStyleName("gerrit-CommentPanel-Last");
+        cp.setOpen(true);
+      }
+      comments.add(cp);
     }
 
     if (msgList.size() > 1) {
-      messagesContent.add(messagesMenuBar());
+      comments.add(messagesMenuBar());
     }
-    messagesPanel.setOpen(msgList.size() > 0);
-    messagesPanel.setVisible(msgList.size() > 0);
+    comments.setVisible(msgList.size() > 0);
   }
 
   private LinkMenuBar messagesMenuBar() {
-    final Panel c = messagesContent;
+    final Panel c = comments;
     final LinkMenuBar m = new LinkMenuBar();
     m.addItem(Util.C.messageExpandRecent(), new ExpandAllCommand(c, true) {
       @Override
-      protected void expand(final ComplexDisclosurePanel w) {
-        final MessagePanel mp = (MessagePanel) w.getContent();
-        w.setOpen(mp.isRecent);
+      protected void expand(final CommentPanel w) {
+        w.setOpen(w.isRecent());
       }
     });
     m.addItem(Util.C.messageExpandAll(), new ExpandAllCommand(c, true));
@@ -362,13 +366,6 @@ public class ChangeScreen extends Screen {
     });
   }
 
-  private static FlowPanel wrap(final Widget w) {
-    final FlowPanel p = new FlowPanel();
-    p.add(w);
-    return p;
-  }
-
-
   public class DashboardKeyCommand extends KeyCommand {
     public DashboardKeyCommand(int mask, char key, String help) {
       super(mask, key, help);

src/main/java/com/google/gerrit/client/ui/CommentPanel.java

@@ -0,0 +1,117 @@
+// Copyright (C) 2009 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.client.ui;
+
+import com.google.gerrit.client.FormatUtil;
+import com.google.gerrit.client.data.AccountInfo;
+import com.google.gwt.event.dom.client.ClickEvent;
+import com.google.gwt.event.dom.client.ClickHandler;
+import com.google.gwt.user.client.ui.Composite;
+import com.google.gwt.user.client.ui.FlexTable;
+import com.google.gwt.user.client.ui.FlowPanel;
+import com.google.gwt.user.client.ui.HasHorizontalAlignment;
+import com.google.gwt.user.client.ui.InlineLabel;
+import com.google.gwt.user.client.ui.Widget;
+import com.google.gwt.user.client.ui.HTMLTable.CellFormatter;
+import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder;
+
+import java.util.Date;
+
+public class CommentPanel extends Composite {
+  private static final int SUMMARY_LENGTH = 75;
+  private final Widget summary;
+  private final Widget content;
+  private boolean recent;
+
+  public CommentPanel(final AccountInfo author, final Date when, String message) {
+    message = message.trim();
+
+    final FlowPanel body = new FlowPanel();
+    initWidget(body);
+    setStyleName("gerrit-CommentPanel");
+
+    summary = new InlineLabel(summarize(message));
+    summary.setStyleName("gerrit-CommentPanel-Summary");
+
+    final FlexTable header = new FlexTable();
+    header.setStyleName("gerrit-CommentPanel-Header");
+    header.addClickHandler(new ClickHandler() {
+      @Override
+      public void onClick(ClickEvent event) {
+        setOpen(!isOpen());
+      }
+    });
+    header.setText(0, 0, FormatUtil.name(author));
+    header.setWidget(0, 1, summary);
+    header.setText(0, 2, FormatUtil.shortFormat(when));
+    final CellFormatter fmt = header.getCellFormatter();
+    fmt.setStyleName(0, 0, "gerrit-CommentPanel-AuthorCell");
+    fmt.setStyleName(0, 1, "gerrit-CommentPanel-SummaryCell");
+    fmt.setStyleName(0, 2, "gerrit-CommentPanel-DateCell");
+    fmt.setHorizontalAlignment(0, 2, HasHorizontalAlignment.ALIGN_RIGHT);
+    fmt.getElement(0, 0).setTitle(FormatUtil.nameEmail(author));
+    fmt.getElement(0, 2).setTitle(FormatUtil.mediumFormat(when));
+    body.add(header);
+
+    content = new SafeHtmlBuilder().append(message).wikify().toBlockWidget();
+    content.setStyleName("gerrit-CommentPanel-Message");
+    content.setVisible(false);
+    body.add(content);
+  }
+
+  private static String summarize(final String message) {
+    if (message.length() < SUMMARY_LENGTH) {
+      return message;
+    }
+
+    int p = 0;
+    final StringBuilder r = new StringBuilder();
+    while (r.length() < SUMMARY_LENGTH) {
+      final int e = message.indexOf(' ', p);
+      if (e < 0) {
+        break;
+      }
+
+      final String word = message.substring(p, e).trim();
+      if (SUMMARY_LENGTH <= r.length() + word.length() + 1) {
+        break;
+      }
+      if (r.length() > 0) {
+        r.append(' ');
+      }
+      r.append(word);
+      p = e + 1;
+    }
+    r.append(" \u2026");
+    return r.toString();
+  }
+
+  public boolean isOpen() {
+    return content.isVisible();
+  }
+
+  public void setOpen(final boolean open) {
+    summary.setVisible(!open);
+    content.setVisible(open);
+  }
+
+  public boolean isRecent() {
+    return recent;
+  }
+
+  public void setRecent(final boolean r) {
+    recent = r;
+  }
+}

src/main/java/com/google/gerrit/client/ui/ExpandAllCommand.java

@@ -18,7 +18,7 @@ import com.google.gwt.user.client.Command;
 import com.google.gwt.user.client.ui.Panel;
 import com.google.gwt.user.client.ui.Widget;
 
-/** Expands all {@link ComplexDisclosurePanel} in a parent panel. */
+/** Expands all {@link CommentPanel} in a parent panel. */
 public class ExpandAllCommand implements Command {
   private final Panel panel;
   protected final boolean open;
@@ -30,13 +30,13 @@ public class ExpandAllCommand implements Command {
 
   public void execute() {
     for (final Widget w : panel) {
-      if (w instanceof ComplexDisclosurePanel) {
-        expand((ComplexDisclosurePanel) w);
+      if (w instanceof CommentPanel) {
+        expand((CommentPanel) w);
       }
     }
   }
 
-  protected void expand(final ComplexDisclosurePanel w) {
+  protected void expand(final CommentPanel w) {
     w.setOpen(open);
   }
 }

src/main/java/com/google/gerrit/public/gerrit.css

@@ -64,8 +64,72 @@
   font-weight: bold;
 }
 
+.gerrit-BlockHeader {
+  font-size: small;
+  font-weight: bold;
+  background: #d4e9a9;
+  padding: 0.2em 0.2em 0.2em 0.5em;
+}
 
-/** TabPanel */
+
+/** CommentPanel **/
+.gerrit-CommentPanel {
+  -moz-border-radius-topleft: 8px;
+  -moz-border-radius-topright: 8px;
+  -moz-border-radius-bottomleft: 0;
+  -moz-border-radius-bottomright: 0;
+  
+  -webkit-border-top-left-radius: 8px;
+  -webkit-border-top-right-radius: 8px;
+  -webkit-border-bottom-left-radius: 0px;
+  -webkit-border-bottom-right-radius: 0px;
+
+  border-top: 1px solid lightgray;
+  border-left: 1px solid lightgray;
+  border-right: 1px solid lightgray;
+}
+.gerrit-CommentPanel-Last {
+  -moz-border-radius-bottomleft: 8px;
+  -moz-border-radius-bottomright: 8px;
+
+  -webkit-border-bottom-left-radius: 8px;
+  -webkit-border-bottom-right-radius: 8px;
+
+  border-bottom: 1px solid lightgray;
+}
+.gerrit-CommentPanel-Header {
+  cursor: pointer;
+  width: 100%;
+}
+.gerrit-CommentPanel-Summary {
+  color: #777777;
+  white-space: nowrap;
+  overflow: hidden;
+}
+.gerrit-CommentPanel-AuthorCell {
+  font-weight: bold;
+  white-space: nowrap;
+}
+.gerrit-CommentPanel-SummaryCell {
+  width: 100%;
+}
+.gerrit-CommentPanel-DateCell {
+  white-space: nowrap;
+}
+.gerrit-CommentPanel-Message {
+  font-size: small;
+  padding-left: 0.5em;
+  padding-right: 0.5em;
+}
+.gerrit-CommentPanel-Message p {
+  margin-top: 0px;
+  margin-bottom: 0px;
+  padding-top: 0.5em;
+  padding-bottom: 0.5em;
+}
+
+
+/** TabPanel **/
 .gwt-TabBar {
   border-bottom: 1px solid black;
 }
@@ -600,18 +664,10 @@
   padding-top: 0.5em;
 }
 
-.gerrit-ChangeScreen .gerrit-ChangeMessages .header {
-  width: 35em;
-}
-.gerrit-ChangeScreen .gerrit-ChangeMessages .complexHeader {
-  width: 5em;
-}
-.gerrit-ChangeScreen .gerrit-ChangeMessages .content {
-  border-top: 3px solid #e3e3e3;
-}
-
-.gerrit-ChangeMessage-Message {
-  font-size: small;
+.gerrit-ChangeComments {
+  margin-left: 0.5em;
+  margin-right: 0.5em;
+  width: 60em;
 }
 
 .gerrit-InfoTable {