Refactor linkage from ChangeScreen2 and its Reviewers widget
Move the loop that splits the reviewer and CC list into the Reviewers widget, eliminating a duplicated block of code. Link the widget with its sibling reviewersText in the init() method rather than set(), matching the pattern used in Labels widget. Change-Id: Iebc74f60781140d7a3b8643aed9f2dbb74e4e889
This commit is contained in:
		@@ -16,13 +16,10 @@ package com.google.gerrit.client.change;
 | 
			
		||||
 | 
			
		||||
import com.google.gerrit.client.FormatUtil;
 | 
			
		||||
import com.google.gerrit.client.Gerrit;
 | 
			
		||||
import com.google.gerrit.client.account.AccountInfo;
 | 
			
		||||
import com.google.gerrit.client.actions.ActionInfo;
 | 
			
		||||
import com.google.gerrit.client.changes.ChangeApi;
 | 
			
		||||
import com.google.gerrit.client.changes.ChangeInfo;
 | 
			
		||||
import com.google.gerrit.client.changes.ChangeInfo.ApprovalInfo;
 | 
			
		||||
import com.google.gerrit.client.changes.ChangeInfo.CommitInfo;
 | 
			
		||||
import com.google.gerrit.client.changes.ChangeInfo.LabelInfo;
 | 
			
		||||
import com.google.gerrit.client.changes.ChangeInfo.MergeableInfo;
 | 
			
		||||
import com.google.gerrit.client.changes.ChangeInfo.MessageInfo;
 | 
			
		||||
import com.google.gerrit.client.changes.ChangeInfo.RevisionInfo;
 | 
			
		||||
@@ -81,9 +78,7 @@ import com.google.gwtorm.client.KeyUtil;
 | 
			
		||||
import java.sql.Timestamp;
 | 
			
		||||
import java.util.ArrayList;
 | 
			
		||||
import java.util.EnumSet;
 | 
			
		||||
import java.util.HashMap;
 | 
			
		||||
import java.util.List;
 | 
			
		||||
import java.util.Map;
 | 
			
		||||
 | 
			
		||||
public class ChangeScreen2 extends Screen {
 | 
			
		||||
  interface Binder extends UiBinder<HTMLPanel, ChangeScreen2> {}
 | 
			
		||||
@@ -208,7 +203,7 @@ public class ChangeScreen2 extends Screen {
 | 
			
		||||
    Resources.I.style().ensureInjected();
 | 
			
		||||
    star.setVisible(Gerrit.isSignedIn());
 | 
			
		||||
    labels.init(style, statusText);
 | 
			
		||||
    reviewers.init(style);
 | 
			
		||||
    reviewers.init(style, reviewersText);
 | 
			
		||||
 | 
			
		||||
    keysNavigation = new KeyCommandSet(Gerrit.C.sectionNavigation());
 | 
			
		||||
    keysNavigation.add(new KeyCommand(0, 'u', Util.C.upToChangeList()) {
 | 
			
		||||
@@ -600,7 +595,6 @@ public class ChangeScreen2 extends Screen {
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    renderOwner(info);
 | 
			
		||||
    renderReviewers(info);
 | 
			
		||||
    renderActionTextDate(info);
 | 
			
		||||
    renderHistory(info);
 | 
			
		||||
    initRevisionsAction(info, revision);
 | 
			
		||||
@@ -618,6 +612,7 @@ public class ChangeScreen2 extends Screen {
 | 
			
		||||
    topic.set(info, revision);
 | 
			
		||||
    commit.set(commentLinkProcessor, info, revision);
 | 
			
		||||
    related.set(info, revision);
 | 
			
		||||
    reviewers.set(info);
 | 
			
		||||
    quickApprove.set(info, revision);
 | 
			
		||||
 | 
			
		||||
    if (Gerrit.isSignedIn()) {
 | 
			
		||||
@@ -645,27 +640,6 @@ public class ChangeScreen2 extends Screen {
 | 
			
		||||
    setWindowTitle(sb.toString());
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  private void renderReviewers(ChangeInfo info) {
 | 
			
		||||
    // TODO Fix approximation of reviewers and CC list(s).
 | 
			
		||||
    Map<Integer, AccountInfo> r = new HashMap<Integer, AccountInfo>();
 | 
			
		||||
    Map<Integer, AccountInfo> cc = new HashMap<Integer, AccountInfo>();
 | 
			
		||||
    for (LabelInfo label : Natives.asList(info.all_labels().values())) {
 | 
			
		||||
      if (label.all() != null) {
 | 
			
		||||
        for (ApprovalInfo ai : Natives.asList(label.all())) {
 | 
			
		||||
          (ai.value() != 0 ? r : cc).put(ai._account_id(), ai);
 | 
			
		||||
        }
 | 
			
		||||
      }
 | 
			
		||||
    }
 | 
			
		||||
    for (Integer i : r.keySet()) {
 | 
			
		||||
      cc.remove(i);
 | 
			
		||||
    }
 | 
			
		||||
    r.remove(info.owner()._account_id());
 | 
			
		||||
    cc.remove(info.owner()._account_id());
 | 
			
		||||
    reviewersText.setInnerSafeHtml(Labels.formatUserList(style, r.values()));
 | 
			
		||||
    reviewers.set(info.legacy_id());
 | 
			
		||||
    reviewers.setReviewers(Labels.formatUserList(style, cc.values()));
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  private void renderOwner(ChangeInfo info) {
 | 
			
		||||
    // TODO info card hover
 | 
			
		||||
    String name = info.owner().name() != null
 | 
			
		||||
 
 | 
			
		||||
@@ -48,7 +48,6 @@ import com.google.gwt.user.client.ui.SuggestBox;
 | 
			
		||||
import com.google.gwt.user.client.ui.SuggestBox.DefaultSuggestionDisplay;
 | 
			
		||||
import com.google.gwt.user.client.ui.SuggestOracle.Suggestion;
 | 
			
		||||
import com.google.gwt.user.client.ui.UIObject;
 | 
			
		||||
import com.google.gwtexpui.safehtml.client.SafeHtml;
 | 
			
		||||
import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder;
 | 
			
		||||
 | 
			
		||||
import java.util.HashMap;
 | 
			
		||||
@@ -59,17 +58,19 @@ class Reviewers extends Composite {
 | 
			
		||||
  interface Binder extends UiBinder<HTMLPanel, Reviewers> {}
 | 
			
		||||
  private static final Binder uiBinder = GWT.create(Binder.class);
 | 
			
		||||
 | 
			
		||||
  @UiField Element ccText;
 | 
			
		||||
  @UiField Button openForm;
 | 
			
		||||
  @UiField Element reviewers;
 | 
			
		||||
  @UiField Element form;
 | 
			
		||||
  @UiField Element error;
 | 
			
		||||
  @UiField(provided = true)
 | 
			
		||||
  SuggestBox suggestBox;
 | 
			
		||||
 | 
			
		||||
  private ChangeScreen2.Style style;
 | 
			
		||||
  private Element reviewersText;
 | 
			
		||||
 | 
			
		||||
  private RestReviewerSuggestOracle reviewerSuggestOracle;
 | 
			
		||||
  private HintTextBox nameTxtBox;
 | 
			
		||||
  private Change.Id changeId;
 | 
			
		||||
  private ChangeScreen2.Style style;
 | 
			
		||||
  private boolean submitOnSelection;
 | 
			
		||||
 | 
			
		||||
  Reviewers() {
 | 
			
		||||
@@ -108,18 +109,16 @@ class Reviewers extends Composite {
 | 
			
		||||
    });
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  void set(Change.Id changeId) {
 | 
			
		||||
    this.changeId = changeId;
 | 
			
		||||
    reviewerSuggestOracle.setChange(changeId);
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  void init(ChangeScreen2.Style style) {
 | 
			
		||||
  void init(ChangeScreen2.Style style, Element reviewersText) {
 | 
			
		||||
    this.style = style;
 | 
			
		||||
    openForm.setVisible(Gerrit.isSignedIn());
 | 
			
		||||
    this.reviewersText = reviewersText;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  void setReviewers(SafeHtml formatUserList) {
 | 
			
		||||
    reviewers.setInnerSafeHtml(formatUserList);
 | 
			
		||||
  void set(ChangeInfo info) {
 | 
			
		||||
    this.changeId = info.legacy_id();
 | 
			
		||||
    display(info);
 | 
			
		||||
    reviewerSuggestOracle.setChange(changeId);
 | 
			
		||||
    openForm.setVisible(Gerrit.isSignedIn());
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  @UiHandler("openForm")
 | 
			
		||||
@@ -219,7 +218,9 @@ class Reviewers extends Composite {
 | 
			
		||||
    for (Integer i : r.keySet()) {
 | 
			
		||||
      cc.remove(i);
 | 
			
		||||
    }
 | 
			
		||||
    r.remove(info.owner()._account_id());
 | 
			
		||||
    cc.remove(info.owner()._account_id());
 | 
			
		||||
    setReviewers(Labels.formatUserList(style, cc.values()));
 | 
			
		||||
    reviewersText.setInnerSafeHtml(Labels.formatUserList(style, r.values()));
 | 
			
		||||
    ccText.setInnerSafeHtml(Labels.formatUserList(style, cc.values()));
 | 
			
		||||
  }
 | 
			
		||||
}
 | 
			
		||||
 
 | 
			
		||||
@@ -44,7 +44,7 @@ limitations under the License.
 | 
			
		||||
  </ui:style>
 | 
			
		||||
  <g:HTMLPanel>
 | 
			
		||||
    <div>
 | 
			
		||||
      <span ui:field='reviewers'/>
 | 
			
		||||
      <span ui:field='ccText'/>
 | 
			
		||||
      <g:Button ui:field='openForm'
 | 
			
		||||
         title='Add reviewers to this change'
 | 
			
		||||
         styleName='{style.openAdd}'
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user