You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "eirikbakke (via GitHub)" <gi...@apache.org> on 2023/01/31 19:51:46 UTC
[GitHub] [netbeans] eirikbakke commented on a diff in pull request #5391: Fix POM Graph colors for dark themes
eirikbakke commented on code in PR #5391:
URL: https://github.com/apache/netbeans/pull/5391#discussion_r1092400107
##########
java/java.graph/src/org/netbeans/modules/java/graph/NodeWidget.java:
##########
@@ -254,16 +264,34 @@ void updatePaintContent() {
foreC = Color.LIGHT_GRAY;
}
if (isDisabled) {
- foreC = new Color ((int)(foreC.getAlpha() / 1.3f), foreC.getRed(),
- foreC.getGreen(), foreC.getBlue());
+ foreC = new Color(foreC.getRed(), foreC.getGreen(), foreC.getBlue(), (int) (foreC.getAlpha() / 1.3f));
}
}
-
- contentW.setBorder(BorderFactory.createLineBorder(10, foreC));
nodeW.setForeground(foreC);
- if(versionW != null) {
+ DependencyGraphScene scene = getDependencyGraphScene();
+ int conflictType = scene.supportsVersions() ? node.getConflictType(scene::isConflict, scene::compareVersions) : VERSION_NO_CONFLICT;
+ if (isHighlighted) {
+ nodeW.getLabelWidget().setForeground(Color.BLACK);
+ } else {
+ nodeW.getLabelWidget().setForeground(origForeground);
+// if (paintState != EdgeWidget.GRAYED) {
Review Comment:
Commented code here should be removed.
##########
java/java.graph/src/org/netbeans/modules/java/graph/NodeWidget.java:
##########
@@ -78,13 +77,18 @@
private static final int RIGHT_TOP = 3;
private static final int RIGHT_BOTTOM = 4;
- private static final @StaticResource String LOCK_ICON = "org/netbeans/modules/java/graph/resources/lock.png";
- private static final @StaticResource String LOCK_BROKEN_ICON = "org/netbeans/modules/java/graph/resources/lock-broken.png";
- private static final @StaticResource String BULB_ICON = "org/netbeans/modules/java/graph/resources/bulb.gif";
- private static final @StaticResource String BULB_HIGHLIGHT_ICON = "org/netbeans/modules/java/graph/resources/bulb-highlight.gif";
+ private static final @StaticResource
+ String LOCK_ICON = "org/netbeans/modules/java/graph/resources/lock.png";
+ private static final @StaticResource
+ String LOCK_BROKEN_ICON = "org/netbeans/modules/java/graph/resources/lock-broken.png";
+ private static final @StaticResource
+ String BULB_ICON = "org/netbeans/modules/java/graph/resources/bulb.gif";
+ private static final @StaticResource
+ String BULB_HIGHLIGHT_ICON = "org/netbeans/modules/java/graph/resources/bulb-highlight.gif";
Review Comment:
These formatting changes can be left out of the patch.
##########
java/java.graph/src/org/netbeans/modules/java/graph/NodeWidget.java:
##########
@@ -102,25 +106,27 @@
private String tooltipText;
private final WidgetAction fixConflictAction;
- private final WidgetAction sceneHoverActionAction;
-
+ private final WidgetAction sceneHoverActionAction;
+
// for use from FruchtermanReingoldLayout
- public double locX, locY, dispX, dispY;
- private boolean fixed;
-
- NodeWidget(@NonNull DependencyGraphScene scene, GraphNode<I> node, final Action fixConflictAction, final WidgetAction sceneHoverActionAction) {
+ public double locX, locY, dispX, dispY;
+ private boolean fixed;
+
+ NodeWidget(@NonNull DependencyGraphScene scene, GraphNode<I> node, final Action fixConflictAction, final WidgetAction sceneHoverActionAction) {
super(scene);
Parameters.notNull("node", node);
- if(fixConflictAction != null) {
+ if (fixConflictAction != null) {
Review Comment:
Nit: Lots of whitespace changes in this commit makes it hard to review the real changes. It's a good idea to put whitespace changes in a separate commit (though no need to change it now).
##########
java/java.graph/src/org/netbeans/modules/java/graph/NodeWidget.java:
##########
@@ -484,50 +511,55 @@ protected void notifyStateChanged(ObjectState previousState, ObjectState state)
enlargedFromHover = false;
}
}
-
+
if (previousState.isSelected() != state.isSelected()) {
updateNeeded = true;
}
if (updateNeeded) {
updateContent();
- } else if (repaintNeeded) {
- repaint();
+ } else {
+ if (repaintNeeded) {
+ repaint();
+ }
}
}
- @Override public void actionPerformed(ActionEvent e) {
+ @Override
+ public void actionPerformed(ActionEvent e) {
enlargedFromHover = true;
updateContent();
}
- public void setReadable (boolean readable) {
+ public void setReadable(boolean readable) {
if (this.readable == readable) {
return;
}
this.readable = readable;
updateContent();
}
- public boolean isReadable () {
+ public boolean isReadable() {
return readable;
}
- public GraphNode getNode () {
+ public GraphNode getNode() {
return node;
}
/**
- * readable widgets are calculated based on scene zoom factor when zoom factor changes, the readable scope should too
+ * readable widgets are calculated based on scene zoom factor when zoom
+ * factor
+ * changes, the readable scope should too
Review Comment:
Unintended formatting change?
##########
java/java.graph/src/org/netbeans/modules/java/graph/NodeWidget.java:
##########
@@ -484,50 +511,55 @@ protected void notifyStateChanged(ObjectState previousState, ObjectState state)
enlargedFromHover = false;
}
}
-
+
if (previousState.isSelected() != state.isSelected()) {
updateNeeded = true;
}
if (updateNeeded) {
updateContent();
- } else if (repaintNeeded) {
- repaint();
+ } else {
+ if (repaintNeeded) {
Review Comment:
Another case of an "else if" becoming a nested if...
##########
java/java.graph/src/org/netbeans/modules/java/graph/NodeWidget.java:
##########
@@ -178,48 +184,52 @@ public String getConflictTooltip(GraphNode<I> node) {
String version = scene.getVersion(firstConflict);
String requester = parent != null ? parent.getName() : "???";
tooltip.append(conflictType == VERSION_CONFLICT ? TIP_SingleConflict(version, requester) : TIP_SingleWarning(version, requester));
- } else if (conflictCount > 1) {
- tooltip.append(conflictType == VERSION_CONFLICT ? TIP_MultipleConflict() : TIP_MultipleWarning());
- for (I nd : node.getDuplicatesOrConflicts()) {
- if (scene.isConflict(nd)) {
- tooltip.append("<tr><td>");
- tooltip.append(scene.getVersion(nd));
- tooltip.append("</td>");
- tooltip.append("<td>");
- GraphNodeImplementation parent = nd.getParent();
- if (parent != null) {
+ } else {
+ if (conflictCount > 1) {
Review Comment:
Why did the "else if" become a separate "if" here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists