Skip to content

Commit 778f681

Browse files
matthijskooijmancmaglie
authored andcommitted
Remove unneeded color-setting code in the boards and library manager
Previously, for the boards manager: - InstallerJDialog would set the "selection background" color on the table, using the "status.notice.bgcolor" the color (default blueish green). This color is not used directly, but made available for cell renderers to use. https://github.com/arduino/Arduino/blob/a1448876a1115c9d3ee9e88f29a15bb081a27816/app/src/cc/arduino/contributions/ui/InstallerJDialog.java#L183 - For each cell, either a ContributedPlatformTableCellEditor or ContributedPlatformTableCellRenderer is used, depending on whether the cell is being edited (i.e. when selected). - Both of these create a ContributedPlatformTableCellJPanel, and call its `update` method, which creates the components for the cell. - The `update` method als sets the background color of the description to white, which does not actually have any effect because the description is not opaque. https://github.com/arduino/Arduino/blob/a1448876a1115c9d3ee9e88f29a15bb081a27816/app/src/cc/arduino/contributions/packages/ui/ContributedPlatformTableCellJPanel.java#L271 https://github.com/arduino/Arduino/blob/a1448876a1115c9d3ee9e88f29a15bb081a27816/app/src/cc/arduino/contributions/packages/ui/ContributedPlatformTableCellJPanel.java#L309 https://docs.oracle.com/javase/7/docs/api/javax/swing/JComponent.html#setBackground(java.awt.Color) - The `update` method also sets its colors of itself (JPanel) to the FG and BG color, or the selected FG and BG color of the table depending on the selected status of the cell. These seem to default to black on white for non-selected and white on blue-ish for selected cells. However, InstallJDialog has replaced the selected BG with a blueish green, as shown above. Of these, only the BG colors actually seem to take effect. The fg color of the description component is actually used (default black). https://github.com/arduino/Arduino/blob/a1448876a1115c9d3ee9e88f29a15bb081a27816/app/src/cc/arduino/contributions/packages/ui/ContributedPlatformTableCellJPanel.java#L282-L288 - After calling `update`, ContributedPlatformTableCellEditor overrides the JPanel background color with a fixed grey color. Similarly, ContributedPlatformTableCellRenderer sets an alternating white and (slightly lighter) grey background color. Together, this means that the background color set by ContributedPlatformTableCellJPanel is never actually used. https://github.com/arduino/Arduino/blob/a1448876a1115c9d3ee9e88f29a15bb081a27816/app/src/cc/arduino/contributions/packages/ui/ContributedPlatformTableCellEditor.java#L132-L133 https://github.com/arduino/Arduino/blob/a1448876a1115c9d3ee9e88f29a15bb081a27816/app/src/cc/arduino/contributions/packages/ui/ContributedPlatformTableCellRenderer.java#L47-L53 For the library manager, pretty much the same happens. Effectively, the only colors that were actually used were the background colors set by ContributedPlatformTableCellEditor and ContributedPlatformTableCellRenderer. This is problematic because: - There is a lot of other confusing and unused code - The foreground color is never set. This is fine when it is black or another dark color, but when the system is configured with a dark theme, the default foreground color will be white, which is problematic on a white background. This commit remove the unneeded code, setting the foreground color is left for later. It also removes the (now unused) `isSelected` from `ContributedPlatformTableCellJPanel::update`. For the library manager, the corresponding argument is still used to decide the "author" color.
1 parent b4bcb30 commit 778f681

5 files changed

+3
-23
lines changed

Diff for: app/src/cc/arduino/contributions/libraries/ui/ContributedLibraryTableCellJPanel.java

-9
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,6 @@ public ContributedLibraryTableCellJPanel(JTable parentTable, Object value,
199199
description.setText(desc);
200200
// copy description to accessibility context for screen readers to use
201201
description.getAccessibleContext().setAccessibleDescription(desc);
202-
description.setBackground(Color.WHITE);
203202

204203
// for modelToView to work, the text area has to be sized. It doesn't
205204
// matter if it's visible or not.
@@ -209,14 +208,6 @@ public ContributedLibraryTableCellJPanel(JTable parentTable, Object value,
209208
InstallerTableCell
210209
.setJTextPaneDimensionToFitContainedText(description,
211210
parentTable.getBounds().width);
212-
213-
if (isSelected) {
214-
setBackground(parentTable.getSelectionBackground());
215-
setForeground(parentTable.getSelectionForeground());
216-
} else {
217-
setBackground(parentTable.getBackground());
218-
setForeground(parentTable.getForeground());
219-
}
220211
}
221212

222213
// same function as in ContributedPlatformTableCellJPanel - is there a utils file this can move to?

Diff for: app/src/cc/arduino/contributions/packages/ui/ContributedPlatformTableCellEditor.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ public Component getTableCellEditorComponent(JTable table, Object _value,
129129
cell.versionToInstallChooser
130130
.setVisible(installed == null && uninstalledReleases.size() > 1);
131131

132-
cell.update(table, _value, true, !installedBuiltIn.isEmpty());
132+
cell.update(table, _value, !installedBuiltIn.isEmpty());
133133
cell.setBackground(new Color(218, 227, 227)); // #dae3e3
134134
return cell;
135135
}

Diff for: app/src/cc/arduino/contributions/packages/ui/ContributedPlatformTableCellJPanel.java

+1-11
Original file line numberDiff line numberDiff line change
@@ -175,8 +175,7 @@ private String setButtonOrLink(JButton button, String desc, String label, String
175175
return retString;
176176
}
177177

178-
void update(JTable parentTable, Object value, boolean isSelected,
179-
boolean hasBuiltInRelease) {
178+
void update(JTable parentTable, Object value, boolean hasBuiltInRelease) {
180179
ContributedPlatformReleases releases = (ContributedPlatformReleases) value;
181180

182181
JTextPane description = makeNewDescription();
@@ -262,7 +261,6 @@ void update(JTable parentTable, Object value, boolean isSelected,
262261
description.setText(desc);
263262
// copy description to accessibility context for screen readers to use
264263
description.getAccessibleContext().setAccessibleDescription(desc);
265-
description.setBackground(Color.WHITE);
266264

267265
// for modelToView to work, the text area has to be sized. It doesn't
268266
// matter if it's visible or not.
@@ -272,14 +270,6 @@ void update(JTable parentTable, Object value, boolean isSelected,
272270
int width = parentTable.getBounds().width;
273271
InstallerTableCell.setJTextPaneDimensionToFitContainedText(description,
274272
width);
275-
276-
if (isSelected) {
277-
setBackground(parentTable.getSelectionBackground());
278-
setForeground(parentTable.getSelectionForeground());
279-
} else {
280-
setBackground(parentTable.getBackground());
281-
setForeground(parentTable.getForeground());
282-
}
283273
}
284274

285275
private JTextPane makeNewDescription() {

Diff for: app/src/cc/arduino/contributions/packages/ui/ContributedPlatformTableCellRenderer.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public Component getTableCellRendererComponent(JTable table, Object value,
4444
int column) {
4545
ContributedPlatformTableCellJPanel cell = new ContributedPlatformTableCellJPanel();
4646
cell.setButtonsVisible(false);
47-
cell.update(table, value, isSelected, false);
47+
cell.update(table, value, false);
4848

4949
if (row % 2 == 0) {
5050
cell.setBackground(new Color(236, 241, 241)); // #ecf1f1

Diff for: app/src/cc/arduino/contributions/ui/InstallerJDialog.java

-1
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,6 @@ public void windowOpened(WindowEvent e) {
180180
contribTable.setDragEnabled(false);
181181
contribTable.setIntercellSpacing(new Dimension(0, 1));
182182
contribTable.setShowVerticalLines(false);
183-
contribTable.setSelectionBackground(Theme.getColor("status.notice.bgcolor"));
184183
contribTable.addKeyListener(new AbstractKeyListener() {
185184

186185
@Override

0 commit comments

Comments
 (0)