From 4cadf95f74f53401b9919dc6ff92c7464c2649e9 Mon Sep 17 00:00:00 2001 From: Nadya Zabrodina Date: Wed, 21 Oct 2015 19:59:48 +0300 Subject: [PATCH] [push]: IDEA-146323 Push dialog distracts by showing many progresses in multi-repo case * remove rendering crutches; * deleted animated gif icons and timer; * add busy icon --- .../intellij/dvcs/push/PushController.java | 56 ++++++++---- .../intellij/dvcs/push/ui/LoadingIcon.java | 86 ------------------- .../com/intellij/dvcs/push/ui/PushLog.java | 16 +++- .../intellij/dvcs/push/ui/RepositoryNode.java | 53 +----------- .../push/ui/RepositoryWithBranchPanel.java | 49 ----------- .../dvcs/push/ui/SingleRepositoryNode.java | 4 - .../src/com/intellij/ui/CheckboxTreeBase.java | 7 +- 7 files changed, 59 insertions(+), 212 deletions(-) delete mode 100644 platform/dvcs-impl/src/com/intellij/dvcs/push/ui/LoadingIcon.java diff --git a/platform/dvcs-impl/src/com/intellij/dvcs/push/PushController.java b/platform/dvcs-impl/src/com/intellij/dvcs/push/PushController.java index 456796d12cea..89c137ae70f2 100644 --- a/platform/dvcs-impl/src/com/intellij/dvcs/push/PushController.java +++ b/platform/dvcs-impl/src/com/intellij/dvcs/push/PushController.java @@ -172,6 +172,9 @@ public class PushController implements Disposable { else if (model.getSupport().shouldRequestIncomingChangesForNotCheckedRepositories() && !repoNode.equals(nodeForCurrentEditor)) { others.put(repoNode, model); } + if (shouldPreSelect(model)) { + model.setChecked(true); + } } if (nodeForCurrentEditor != null) { //add repo for currently opened editor to the end of priority queue @@ -181,7 +184,12 @@ public class PushController implements Disposable { loadCommitsFromMap(others); } - @Nullable + private boolean shouldPreSelect(@NotNull MyRepoModel model) { + Repository repository = model.getRepository(); + return mySingleRepoProject || preselectByUser(repository) || + (notExcludedByUser(repository) && model.getSupport().shouldRequestIncomingChangesForNotCheckedRepositories()); + } + private RepositoryNode findNodeByRepo(@Nullable final Repository repository) { if (repository == null) return null; Map.Entry> entry = @@ -321,7 +329,7 @@ public class PushController implements Disposable { Collection nodes = getNodesForSupport(pushSupport); if (hasSomethingToPush(nodes)) return true; if (hasCheckedNodesWithContent(nodes, force || myDialog.getAdditionalOptionValue(pushSupport) != null)) { - return !pushSupport.getRepositoryManager().isSyncEnabled() || allNodesAreLoaded(nodes); + return !pushSupport.getRepositoryManager().isSyncEnabled() || !hasLoadingNodes(nodes); } return false; } @@ -332,7 +340,7 @@ public class PushController implements Disposable { public boolean value(@NotNull RepositoryNode node) { PushTarget target = myView2Model.get(node).getTarget(); //if node is selected target should not be null - return (node.isChecked() || node.isLoading()) && target != null && target.hasSomethingToPush(); + return node.isChecked() && target != null && target.hasSomethingToPush(); } }); } @@ -357,9 +365,8 @@ public class PushController implements Disposable { }); } - - private static boolean allNodesAreLoaded(@NotNull Collection nodes) { - return !ContainerUtil.exists(nodes, new Condition() { + private static boolean hasLoadingNodes(@NotNull Collection nodes) { + return ContainerUtil.exists(nodes, new Condition() { @Override public boolean value(@NotNull RepositoryNode node) { return node.isLoading(); @@ -404,6 +411,7 @@ public class PushController implements Disposable { error.handleError(new CommitLoader() { @Override public void reloadCommits() { + node.setChecked(true); loadCommits(model, node, false); } }); @@ -412,11 +420,14 @@ public class PushController implements Disposable { return new TextWithLinkNode(errorLinkText); } })); + if (node.isChecked()) { + node.setChecked(false); + } } else { List commits = outgoing.getCommits(); model.setLoadedCommits(commits); - shouldBeSelected = shouldSelect(model); + shouldBeSelected = shouldSelectNodeAfterLoad(model); myPushLog.setChildren(node, getPresentationForCommits(PushController.this.myProject, model.getLoadedCommits(), model.getNumberOfShownCommits())); @@ -425,21 +436,30 @@ public class PushController implements Disposable { } } node.stopLoading(); - if (shouldBeSelected) { // never remove selection; initially all checkboxes are not selected + updateLoadingPanel(); + if (shouldBeSelected) { node.setChecked(true); } + else if (initial) { + //do not un-check if user checked manually and no errors occurred, only initial check may be changed + node.setChecked(false); + } myDialog.updateOkActions(); } }); } }; node.startLoading(myPushLog.getTree(), myExecutorService.submit(task, result), initial); + updateLoadingPanel(); } - private boolean shouldSelect(@NotNull MyRepoModel model) { + private void updateLoadingPanel() { + myPushLog.getTree().setPaintBusy(hasLoadingNodes(myView2Model.keySet())); + } + + private boolean shouldSelectNodeAfterLoad(@NotNull MyRepoModel model) { if (mySingleRepoProject) return true; - Repository repository = model.getRepository(); - return hasCommitsToPush(model) && (preselectByUser(repository) || notExcludedByUser(repository)); + return hasCommitsToPush(model) && model.isSelected(); } private boolean notExcludedByUser(@NotNull Repository repository) { @@ -503,18 +523,14 @@ public class PushController implements Disposable { if (mySingleRepoProject) { return myView2Model.values(); } - //return not only selected but all loading with something to push; - // otherwise push/forcepush button may be enabled but no selected node exists + //return all selected despite a loading state; return ContainerUtil.mapNotNull(myView2Model.entrySet(), new Function>, MyRepoModel>() { @Override public MyRepoModel fun(Map.Entry> entry) { MyRepoModel model = entry.getValue(); - return model.isSelected() || - (entry.getKey().isLoading() && - model.getTarget() != null && - notExcludedByUser(model.getRepository()) && - model.getTarget().hasSomethingToPush()) ? model : + return model.isSelected() && + model.getTarget() != null ? model : null; } }); @@ -697,6 +713,10 @@ public class PushController implements Disposable { public CheckBoxModel getCheckBoxModel() { return myCheckBoxModel; } + + public void setChecked(boolean checked) { + myCheckBoxModel.setChecked(checked); + } } private static class MyDoNotAskOptionForPush implements DialogWrapper.DoNotAskOption { diff --git a/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/LoadingIcon.java b/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/LoadingIcon.java deleted file mode 100644 index b983e8f46024..000000000000 --- a/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/LoadingIcon.java +++ /dev/null @@ -1,86 +0,0 @@ -/* - * Copyright 2000-2014 JetBrains s.r.o. - * - * 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.intellij.dvcs.push.ui; - -import com.intellij.openapi.diagnostic.Logger; -import com.intellij.util.ImageLoader; -import com.intellij.util.ui.JBImageIcon; -import com.intellij.util.ui.UIUtil; -import org.jetbrains.annotations.NotNull; - -import javax.swing.*; -import javax.swing.tree.DefaultTreeModel; -import javax.swing.tree.TreeNode; -import javax.swing.tree.TreePath; -import java.awt.*; -import java.awt.image.ImageObserver; - -class LoadingIcon extends JBImageIcon { - - //todo fix double size animated icons - private static final String LOADING_ICON = "/icons/loading.gif"; - private static final Logger LOG = Logger.getInstance(LoadingIcon.class); - - LoadingIcon(@NotNull Image image) { - super(image); - } - - @NotNull - static LoadingIcon create(int width, int height) { - Image image = ImageLoader.loadFromResource(LOADING_ICON); - if (image == null) { - LOG.error("Couldn't load image: " + LOADING_ICON); - return createEmpty(width, height); - } - return new LoadingIcon(image); - } - - @NotNull - static LoadingIcon createEmpty(int width, int height) { - return new LoadingIcon(UIUtil.createImage(width, height, Transparency.TRANSLUCENT)); - } - - void setObserver(@NotNull JTree tree, @NotNull TreeNode treeNode) { - setImageObserver(new NodeImageObserver(tree, treeNode)); - } - - private static class NodeImageObserver implements ImageObserver { - @NotNull private final JTree myTree; - @NotNull private final DefaultTreeModel myModel; - @NotNull private final TreeNode myNode; - - NodeImageObserver(@NotNull JTree tree, @NotNull TreeNode node) { - myTree = tree; - myModel = (DefaultTreeModel)tree.getModel(); - myNode = node; - } - - public boolean imageUpdate(Image img, int flags, int x, int y, int w, int h) { - if (myNode instanceof RepositoryNode && !((RepositoryNode)myNode).isLoading()) return false; - if ((flags & (FRAMEBITS | ALLBITS)) != 0) { - TreeNode[] pathToRoot = myModel.getPathToRoot(myNode); - if (pathToRoot != null) { - TreePath path = new TreePath(pathToRoot); - Rectangle rect = myTree.getPathBounds(path); - if (rect != null) { - myTree.repaint(rect); - } - } - } - return (flags & (ALLBITS | ABORT)) == 0; - } - } -} diff --git a/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/PushLog.java b/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/PushLog.java index e10c4bacf915..03122730ba39 100644 --- a/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/PushLog.java +++ b/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/PushLog.java @@ -37,6 +37,7 @@ import com.intellij.util.ArrayUtil; import com.intellij.util.Function; import com.intellij.util.Processor; import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.ui.ThreeStateCheckBox; import com.intellij.util.ui.tree.TreeUtil; import com.intellij.util.ui.tree.WideSelectionTreeUI; import com.intellij.vcs.log.Hash; @@ -79,6 +80,10 @@ public class PushLog extends JPanel implements DataProvider { myTreeCellRenderer = new MyTreeCellRenderer(); myTree = new CheckboxTree(myTreeCellRenderer, root) { + protected boolean shouldShowBusyIconIfNeeded() { + return true; + } + public boolean isPathEditable(TreePath path) { return isEditable() && path.getLastPathComponent() instanceof DefaultMutableTreeNode; } @@ -477,7 +482,7 @@ public class PushLog extends JPanel implements DataProvider { } @NotNull - public JTree getTree() { + public CheckboxTree getTree() { return myTree; } @@ -569,7 +574,14 @@ public class PushLog extends JPanel implements DataProvider { // null border works as expected always. if (value instanceof RepositoryNode) { //todo simplify, remove instance of - myCheckbox.setVisible(((RepositoryNode)value).isCheckboxVisible()); + RepositoryNode valueNode = (RepositoryNode)value; + myCheckbox.setVisible(valueNode.isCheckboxVisible()); + if (valueNode.isChecked() && valueNode.isLoading()) { + myCheckbox.setState(ThreeStateCheckBox.State.DONT_CARE); + } + else { + myCheckbox.setSelected(valueNode.isChecked()); + } } Object userObject = ((DefaultMutableTreeNode)value).getUserObject(); ColoredTreeCellRenderer renderer = getTextRenderer(); diff --git a/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/RepositoryNode.java b/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/RepositoryNode.java index 675af3a40366..a4cb4fcf9373 100644 --- a/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/RepositoryNode.java +++ b/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/RepositoryNode.java @@ -27,25 +27,17 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import javax.swing.*; -import javax.swing.border.EmptyBorder; -import java.awt.event.ActionEvent; -import java.awt.event.ActionListener; import java.util.concurrent.Future; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; public class RepositoryNode extends CheckedTreeNode implements EditableTreeNode, Comparable { - private static final int PROGRESS_DELAY = 100; - private static final int START_DELAY = 500; - @NotNull private final LoadingIcon myLoadingIcon; @NotNull protected final AtomicBoolean myLoading = new AtomicBoolean(); @NotNull private final CheckBoxModel myCheckBoxModel; @NotNull private final RepositoryWithBranchPanel myRepositoryPanel; @Nullable private Future> myFuture; - private final int myCheckBoxHGap; - private final int myCheckBoxVGap; public RepositoryNode(@NotNull RepositoryWithBranchPanel repositoryPanel, @NotNull CheckBoxModel model, boolean enabled) { super(repositoryPanel); @@ -53,9 +45,6 @@ public class RepositoryNode extends CheckedTreeNode implements EditableTreeNode, setChecked(false); setEnabled(enabled); myRepositoryPanel = repositoryPanel; - myLoadingIcon = myRepositoryPanel.getLoadingIcon(); - myCheckBoxHGap = myRepositoryPanel.getLoadingIconAndCheckBoxGapH(); - myCheckBoxVGap = myRepositoryPanel.getLoadingIconAndCheckBoxGapV(); } @Override @@ -69,7 +58,7 @@ public class RepositoryNode extends CheckedTreeNode implements EditableTreeNode, } public boolean isCheckboxVisible() { - return !myLoading.get(); + return true; } public void forceUpdateUiModelWithTypedText(@NotNull String forceText) { @@ -83,23 +72,6 @@ public class RepositoryNode extends CheckedTreeNode implements EditableTreeNode, public void render(@NotNull ColoredTreeCellRenderer renderer, @Nullable String syncEditingText) { int repoFixedWidth = 120; - int borderHOffset = myRepositoryPanel.getHBorderOffset(renderer); - if (myLoading.get()) { - renderer.setIcon(myLoadingIcon); - renderer.setIconOnTheRight(false); - int checkBoxWidth = myRepositoryPanel.getCheckBoxWidth(); - repoFixedWidth += checkBoxWidth; - if (myCheckBoxHGap > 0) { - renderer.append(""); - renderer.appendTextPadding(checkBoxWidth + renderer.getIconTextGap() + borderHOffset); - } - } - else { - if (myCheckBoxHGap <= 0) { - renderer.append(""); - renderer.appendTextPadding(myRepositoryPanel.calculateRendererShiftH(renderer)); - } - } SimpleTextAttributes repositoryDetailsTextAttributes = PushLogTreeUtil .addTransparencyIfNeeded(SimpleTextAttributes.REGULAR_ATTRIBUTES, isChecked()); @@ -109,15 +81,6 @@ public class RepositoryNode extends CheckedTreeNode implements EditableTreeNode, renderer.append(myRepositoryPanel.getArrow(), repositoryDetailsTextAttributes); PushTargetPanel pushTargetPanel = myRepositoryPanel.getTargetPanel(); pushTargetPanel.render(renderer, renderer.getTree().isPathSelected(TreeUtil.getPathFromRoot(this)), isChecked(), syncEditingText); - - int maxSize = Math.max(myRepositoryPanel.getCheckBoxHeight(), myLoadingIcon.getIconHeight()); - int rendererHeight = renderer.getPreferredSize().height; - if (maxSize > rendererHeight) { - if (myCheckBoxVGap > 0 && isLoading() || myCheckBoxVGap < 0 && !isLoading()) { - int vShift = maxSize - rendererHeight; - renderer.setBorder(new EmptyBorder((vShift + 1) / 2, 0, (vShift) / 2, 0)); - } - } } @NotNull @@ -156,18 +119,7 @@ public class RepositoryNode extends CheckedTreeNode implements EditableTreeNode, @Override public void startLoading(@NotNull final JTree tree, @NotNull Future> future, boolean initial) { myFuture = future; - final Timer t = new Timer(initial ? START_DELAY : PROGRESS_DELAY, new ActionListener() { - @Override - public void actionPerformed(ActionEvent e) { - if (!myFuture.isDone()) { - myLoading.set(true); - myLoadingIcon.setObserver(tree, RepositoryNode.this); - tree.repaint(); - } - } - }); - t.setRepeats(false); - t.start(); + myLoading.set(true); } @Override @@ -183,7 +135,6 @@ public class RepositoryNode extends CheckedTreeNode implements EditableTreeNode, public void stopLoading() { myLoading.set(false); - myLoadingIcon.setImageObserver(null); } public boolean isLoading() { diff --git a/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/RepositoryWithBranchPanel.java b/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/RepositoryWithBranchPanel.java index e2cd7f9c57b6..25a0ae17741c 100644 --- a/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/RepositoryWithBranchPanel.java +++ b/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/RepositoryWithBranchPanel.java @@ -23,7 +23,6 @@ import com.intellij.openapi.ui.MessageType; import com.intellij.openapi.ui.ValidationInfo; import com.intellij.openapi.ui.popup.util.PopupUtil; import com.intellij.ui.ColoredTreeCellRenderer; -import com.intellij.ui.SimpleColoredComponent; import com.intellij.ui.SimpleTextAttributes; import com.intellij.ui.components.JBCheckBox; import com.intellij.ui.components.JBLabel; @@ -33,7 +32,6 @@ import com.intellij.util.ui.UIUtil; import org.jetbrains.annotations.NotNull; import javax.swing.*; -import javax.swing.border.Border; import java.awt.*; import java.awt.event.ActionEvent; import java.awt.event.ActionListener; @@ -48,12 +46,6 @@ public class RepositoryWithBranchPanel extends NonOpaquePa private final JLabel myRepositoryLabel; private final ColoredTreeCellRenderer myTextRenderer; @NotNull private final List> myListeners = ContainerUtil.createLockFreeCopyOnWriteList(); - private final int myCheckBoxLoadingIconGapH; - private final int myCheckBoxLoadingIconGapV; - private final LoadingIcon myLoadingIcon; - private final int myCheckBoxWidth; - private final int myCheckBoxHeight; - public RepositoryWithBranchPanel(@NotNull final Project project, @NotNull String repoName, @NotNull String sourceName, @NotNull PushTargetPanel destPushTargetPanelComponent) { @@ -101,12 +93,6 @@ public class RepositoryWithBranchPanel extends NonOpaquePa JCheckBox emptyBorderCheckBox = new JCheckBox(); emptyBorderCheckBox.setBorder(null); - Dimension size = emptyBorderCheckBox.getPreferredSize(); - myCheckBoxWidth = size.width; - myCheckBoxHeight = size.height; - myLoadingIcon = LoadingIcon.create(myCheckBoxWidth, size.height); - myCheckBoxLoadingIconGapH = myCheckBoxWidth - myLoadingIcon.getIconWidth(); - myCheckBoxLoadingIconGapV = size.height - myLoadingIcon.getIconHeight(); } private void layoutComponents() { @@ -145,10 +131,6 @@ public class RepositoryWithBranchPanel extends NonOpaquePa RepositoryNode node = (RepositoryNode)value; myRepositoryCheckbox.setSelected(node.isChecked()); myRepositoryCheckbox.setVisible(true); - if (myCheckBoxLoadingIconGapH < 0) { - myTextRenderer.append(""); - myTextRenderer.appendTextPadding(calculateRendererShiftH(myTextRenderer)); - } myTextRenderer.append(getRepositoryName(), SimpleTextAttributes.GRAY_ATTRIBUTES); myTextRenderer.appendTextPadding(120); } @@ -204,37 +186,6 @@ public class RepositoryWithBranchPanel extends NonOpaquePa return myDestPushTargetPanelComponent; } - public LoadingIcon getLoadingIcon() { - return myLoadingIcon; - } - - public int getCheckBoxWidth() { - return myCheckBoxWidth; - } - - - public int getLoadingIconAndCheckBoxGapH() { - return myCheckBoxLoadingIconGapH; - } - - public int calculateRendererShiftH(@NotNull SimpleColoredComponent coloredRenderer) { - int borderOffset = getHBorderOffset(coloredRenderer); - return -myCheckBoxLoadingIconGapH + coloredRenderer.getIconTextGap() + coloredRenderer.getIpad().left + borderOffset; - } - - public int getHBorderOffset(@NotNull SimpleColoredComponent coloredRenderer) { - Border border = coloredRenderer.getMyBorder(); - return border != null ? border.getBorderInsets(coloredRenderer).left : 0; - } - - public int getLoadingIconAndCheckBoxGapV() { - return myCheckBoxLoadingIconGapV; - } - - public int getCheckBoxHeight() { - return myCheckBoxHeight; - } - public boolean isEditable() { return myDestPushTargetPanelComponent.getValue() != null; } diff --git a/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/SingleRepositoryNode.java b/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/SingleRepositoryNode.java index b4ad334aca03..8d1e2743dff9 100644 --- a/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/SingleRepositoryNode.java +++ b/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/SingleRepositoryNode.java @@ -45,10 +45,6 @@ public class SingleRepositoryNode extends RepositoryNode { @Override public void render(@NotNull ColoredTreeCellRenderer renderer) { - if (myLoading.get()) { - renderer.setIcon(myRepositoryPanel.getLoadingIcon()); - renderer.setIconOnTheRight(true); - } renderer.append(" "); renderer.append(myRepositoryPanel.getSourceName(), SimpleTextAttributes.REGULAR_ATTRIBUTES); renderer.append(myRepositoryPanel.getArrow(), SimpleTextAttributes.REGULAR_ATTRIBUTES); diff --git a/platform/platform-api/src/com/intellij/ui/CheckboxTreeBase.java b/platform/platform-api/src/com/intellij/ui/CheckboxTreeBase.java index 1a5e4703ec8a..eac57582a39f 100644 --- a/platform/platform-api/src/com/intellij/ui/CheckboxTreeBase.java +++ b/platform/platform-api/src/com/intellij/ui/CheckboxTreeBase.java @@ -17,6 +17,7 @@ package com.intellij.ui; import com.intellij.ui.treeStructure.Tree; import com.intellij.util.EventDispatcher; +import com.intellij.util.ui.ThreeStateCheckBox; import com.intellij.util.ui.UIUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -129,7 +130,7 @@ public class CheckboxTreeBase extends Tree { public static class CheckboxTreeCellRendererBase extends JPanel implements TreeCellRenderer { private final ColoredTreeCellRenderer myTextRenderer; - public final JCheckBox myCheckbox; + public final ThreeStateCheckBox myCheckbox; private final boolean myUsePartialStatusForParentNodes; public CheckboxTreeCellRendererBase(boolean opaque) { @@ -139,7 +140,9 @@ public class CheckboxTreeBase extends Tree { public CheckboxTreeCellRendererBase(boolean opaque, final boolean usePartialStatusForParentNodes) { super(new BorderLayout()); myUsePartialStatusForParentNodes = usePartialStatusForParentNodes; - myCheckbox = new JCheckBox(); + myCheckbox = new ThreeStateCheckBox(); + myCheckbox.setSelected(false); + myCheckbox.setThirdStateEnabled(false); myTextRenderer = new ColoredTreeCellRenderer() { public void customizeCellRenderer(JTree tree, Object value, boolean selected, boolean expanded, boolean leaf, int row, boolean hasFocus) { } };