From c5701ee0fad67713a0d98afd3c91de8b3b597012 Mon Sep 17 00:00:00 2001 From: Christopher Hermann Date: Fri, 16 Feb 2024 10:50:44 +0100 Subject: [PATCH] Highlighting problem when using the dark theme on Windows eclipse-platform/eclipse.platform.swt#811 JFace viewer are using OS selection color to highlight the selected item. On some OS this is not accessible. With this change, the selection color can be changed via color preference in the settings of eclipse. The colors are used to draw selection color for tree and table viewers. For Windows, the selection color in the dark theme is overwritten to fix the bad default coloring Fixes https://github.com/eclipse-platform/eclipse.platform.swt/issues/811 Fixes #1688 --- .../META-INF/MANIFEST.MF | 1 + .../internal/text/TableOwnerDrawSupport.java | 84 +++++++++++++++-- .../CompletionTableDrawSupport.java | 63 +++++++++++++ .../CompletionProposalPopup2.java | 6 +- .../CompletionProposalPopup.java | 6 +- .../FocusCellOwnerDrawHighlighter.java | 15 ++- .../jface/viewers/OwnerDrawLabelProvider.java | 33 ++++--- .../viewers/StyledCellLabelProvider.java | 1 + .../eclipse/jface/viewers/TableViewer.java | 11 +++ .../org/eclipse/jface/viewers/TreeViewer.java | 11 +++ .../ColumnViewerSelectionColorListener.java | 83 +++++++++++++++++ .../css/dark/e4-dark_preferencestyle.css | 2 + .../org.eclipse.ui.themes/css/e4-dark_win.css | 6 ++ .../ColumnViewerSelectionColorFactory.java | 63 +++++++++++++ bundles/org.eclipse.ui/plugin.properties | 9 ++ bundles/org.eclipse.ui/plugin.xml | 68 ++++++++++++++ .../META-INF/MANIFEST.MF | 3 +- .../text/TableOwnerDrawSupportTest.java | 93 +++++++++++++++++++ .../CompletionTableDrawSupportTest.java | 92 ++++++++++++++++++ .../jface/text/tests/JFaceTextTestSuite.java | 8 +- 20 files changed, 624 insertions(+), 34 deletions(-) create mode 100644 bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/contentassist/CompletionTableDrawSupport.java create mode 100644 bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/internal/ColumnViewerSelectionColorListener.java create mode 100644 bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/themes/ColumnViewerSelectionColorFactory.java create mode 100644 tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/internal/text/TableOwnerDrawSupportTest.java create mode 100644 tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/internal/text/contentassist/CompletionTableDrawSupportTest.java diff --git a/bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF b/bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF index 23023cb5a81..58e89a789d3 100644 --- a/bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF @@ -10,6 +10,7 @@ Export-Package: org.eclipse.jface.contentassist.images, org.eclipse.jface.internal.text;x-internal:=true, org.eclipse.jface.internal.text.codemining;x-internal:=true, + org.eclipse.jface.internal.text.contentassist;x-internal:=true, org.eclipse.jface.internal.text.html;x-friends:="org.eclipse.ant.ui, org.eclipse.jdt.ui, org.eclipse.ltk.ui.refactoring, org.eclipse.pde.ui, org.eclipse.ui.editors, org.eclipse.xtext.ui", org.eclipse.jface.internal.text.link.contentassist;x-internal:=true, org.eclipse.jface.internal.text.revisions;x-internal:=true, diff --git a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/TableOwnerDrawSupport.java b/bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/TableOwnerDrawSupport.java index a3052c84d42..7a180c7824b 100644 --- a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/TableOwnerDrawSupport.java +++ b/bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/TableOwnerDrawSupport.java @@ -25,6 +25,9 @@ import org.eclipse.swt.widgets.Table; import org.eclipse.swt.widgets.TableItem; +import org.eclipse.jface.resource.ColorRegistry; +import org.eclipse.jface.resource.JFaceResources; + /** * Adds owner draw support for tables. @@ -42,6 +45,10 @@ public class TableOwnerDrawSupport implements Listener { public static void install(Table table) { TableOwnerDrawSupport listener= new TableOwnerDrawSupport(table); + installListener(table, listener); + } + + protected static void installListener(Table table, Listener listener) { table.addListener(SWT.Dispose, listener); table.addListener(SWT.MeasureItem, listener); table.addListener(SWT.EraseItem, listener); @@ -70,7 +77,7 @@ private static StyleRange[] getStyledRanges(TableItem item, int column) { return (StyleRange[])item.getData(STYLED_RANGES_KEY + column); } - private TableOwnerDrawSupport(Table table) { + protected TableOwnerDrawSupport(Table table) { int orientation= table.getStyle() & (SWT.LEFT_TO_RIGHT | SWT.RIGHT_TO_LEFT); fSharedLayout= new TextLayout(table.getDisplay()); fSharedLayout.setOrientation(orientation); @@ -147,7 +154,28 @@ private void performPaint(Event event) { Color oldForeground= gc.getForeground(); Color oldBackground= gc.getBackground(); - if (!isSelected) { + if (isSelected) { + Color background= item.getParent().isFocusControl() + ? getSelectedRowBackgroundColor() + : getSelectedRowBackgroundColorNoFocus(); + Color foreground= item.getParent().isFocusControl() + ? getSelectedRowForegroundColor() + : getSelectedRowForegroundColorNoFocus(); + + if (background == null) { + background= item.getDisplay().getSystemColor( + SWT.COLOR_LIST_SELECTION); + } + + if (foreground == null) { + foreground= item.getDisplay().getSystemColor( + SWT.COLOR_LIST_SELECTION_TEXT); + } + + gc.setBackground(background); + gc.setForeground(foreground); + gc.fillRectangle(0, event.y, item.getParent().getBounds().width, event.height); + } else { Color foreground= item.getForeground(index); gc.setForeground(foreground); @@ -178,10 +206,54 @@ private void performPaint(Event event) { gc.drawFocus(focusBounds.x, focusBounds.y, focusBounds.width + fDeltaOfLastMeasure, focusBounds.height); } - if (!isSelected) { - gc.setForeground(oldForeground); - gc.setBackground(oldBackground); - } + gc.setForeground(oldForeground); + gc.setBackground(oldBackground); + } + + /** + * The color to use when rendering the background of the selected row when the control has the + * input focus + * + * @return the color or null to use the default + */ + protected Color getSelectedRowBackgroundColor() { + ColorRegistry colorRegistry= JFaceResources.getColorRegistry(); + return colorRegistry.get("org.eclipse.ui.workbench.SELECTED_CELL_BACKGROUND"); //$NON-NLS-1$ + } + + /** + * The color to use when rendering the foreground (=text) of the selected row when the control + * has the input focus + * + * @return the color or null to use the default + */ + protected Color getSelectedRowForegroundColor() { + ColorRegistry colorRegistry= JFaceResources.getColorRegistry(); + return colorRegistry.get("org.eclipse.ui.workbench.SELECTED_CELL_FOREGROUND"); //$NON-NLS-1$ + } + + /** + * The color to use when rendering the foreground (=text) of the selected row when the control + * has no input focus + * + * @return the color or null to use the same used when control has focus + * @since 3.4 + */ + protected Color getSelectedRowForegroundColorNoFocus() { + ColorRegistry colorRegistry= JFaceResources.getColorRegistry(); + return colorRegistry.get("org.eclipse.ui.workbench.SELECTED_CELL_FOREGROUND_NO_FOCUS"); //$NON-NLS-1$ + } + + /** + * The color to use when rendering the background of the selected row when the control has + * no input focus + * + * @return the color or null to use the same used when control has focus + * @since 3.4 + */ + protected Color getSelectedRowBackgroundColorNoFocus() { + ColorRegistry colorRegistry= JFaceResources.getColorRegistry(); + return colorRegistry.get("org.eclipse.ui.workbench.SELECTED_CELL_BACKGROUND_NO_FOCUS"); //$NON-NLS-1$ } private void widgetDisposed() { diff --git a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/contentassist/CompletionTableDrawSupport.java b/bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/contentassist/CompletionTableDrawSupport.java new file mode 100644 index 00000000000..611397a4723 --- /dev/null +++ b/bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/contentassist/CompletionTableDrawSupport.java @@ -0,0 +1,63 @@ +/******************************************************************************* + * Copyright (c) 2024 SAP SE. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * SAP SE - initial API and implementation + *******************************************************************************/ +package org.eclipse.jface.internal.text.contentassist; + +import org.eclipse.swt.custom.StyleRange; +import org.eclipse.swt.graphics.Color; +import org.eclipse.swt.widgets.Table; +import org.eclipse.swt.widgets.TableItem; + +import org.eclipse.jface.internal.text.TableOwnerDrawSupport; + +/** + * When a completion table (for example for code completion) is requested by the user, the user + * needs to be able to continue typing in the linked text control. In such cases, the focus is not + * on the completion table. To ensure the selected code completion proposal always displays in the + * correct color, even if the completion table is not focused, the non-focused colors are overridden + * with the focus colors. + */ +public class CompletionTableDrawSupport extends TableOwnerDrawSupport { + + public static void install(Table table) { + TableOwnerDrawSupport listener= new CompletionTableDrawSupport(table); + installListener(table, listener); + } + + /** + * Stores the styled ranges in the given table item. See {@link TableOwnerDrawSupport} + * + * @param item table item + * @param column the column index + * @param ranges the styled ranges or null to remove them + */ + public static void storeStyleRanges(TableItem item, int column, StyleRange[] ranges) { + TableOwnerDrawSupport.storeStyleRanges(item, column, ranges); + } + + + private CompletionTableDrawSupport(Table table) { + super(table); + } + + @Override + protected Color getSelectedRowBackgroundColorNoFocus() { + return super.getSelectedRowBackgroundColor(); + } + + @Override + protected Color getSelectedRowForegroundColorNoFocus() { + return super.getSelectedRowForegroundColor(); + } + +} diff --git a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/link/contentassist/CompletionProposalPopup2.java b/bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/link/contentassist/CompletionProposalPopup2.java index 6ca009a07d7..067e233936f 100644 --- a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/link/contentassist/CompletionProposalPopup2.java +++ b/bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/link/contentassist/CompletionProposalPopup2.java @@ -39,7 +39,7 @@ import org.eclipse.swt.widgets.Table; import org.eclipse.swt.widgets.TableItem; -import org.eclipse.jface.internal.text.TableOwnerDrawSupport; +import org.eclipse.jface.internal.text.contentassist.CompletionTableDrawSupport; import org.eclipse.jface.preference.JFacePreferences; import org.eclipse.jface.resource.ColorRegistry; import org.eclipse.jface.resource.JFaceColors; @@ -269,7 +269,7 @@ private void createProposalSelector() { fIsColoredLabelsSupportEnabled= fContentAssistant.isColoredLabelsSupportEnabled(); if (fIsColoredLabelsSupportEnabled) - TableOwnerDrawSupport.install(fProposalTable); + CompletionTableDrawSupport.install(fProposalTable); fProposalTable.setLocation(0, 0); if (fAdditionalInfoController != null) @@ -572,7 +572,7 @@ private void setProposals(ICompletionProposal[] proposals) { item.setText(displayString); if (fIsColoredLabelsSupportEnabled) - TableOwnerDrawSupport.storeStyleRanges(item, 0, styleRanges); + CompletionTableDrawSupport.storeStyleRanges(item, 0, styleRanges); item.setData(p); diff --git a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/CompletionProposalPopup.java b/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/CompletionProposalPopup.java index f426f8b7ce8..7297722a77b 100644 --- a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/CompletionProposalPopup.java +++ b/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/CompletionProposalPopup.java @@ -78,7 +78,7 @@ import org.eclipse.jface.bindings.keys.SWTKeySupport; import org.eclipse.jface.contentassist.IContentAssistSubjectControl; import org.eclipse.jface.internal.text.InformationControlReplacer; -import org.eclipse.jface.internal.text.TableOwnerDrawSupport; +import org.eclipse.jface.internal.text.contentassist.CompletionTableDrawSupport; import org.eclipse.jface.preference.JFacePreferences; import org.eclipse.jface.resource.JFaceColors; import org.eclipse.jface.resource.JFaceResources; @@ -613,7 +613,7 @@ void createProposalSelector() { fIsColoredLabelsSupportEnabled= fContentAssistant.isColoredLabelsSupportEnabled(); if (fIsColoredLabelsSupportEnabled) - TableOwnerDrawSupport.install(fProposalTable); + CompletionTableDrawSupport.install(fProposalTable); fProposalTable.setLocation(0, 0); if (fAdditionalInfoController != null) @@ -904,7 +904,7 @@ private void handleSetData(Event event) { item.setText(displayString); if (fIsColoredLabelsSupportEnabled) - TableOwnerDrawSupport.storeStyleRanges(item, 0, styleRanges); + CompletionTableDrawSupport.storeStyleRanges(item, 0, styleRanges); item.setImage(image); item.setData(current); diff --git a/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/FocusCellOwnerDrawHighlighter.java b/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/FocusCellOwnerDrawHighlighter.java index 99925bdc6f9..5455e92aac3 100644 --- a/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/FocusCellOwnerDrawHighlighter.java +++ b/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/FocusCellOwnerDrawHighlighter.java @@ -18,6 +18,8 @@ import org.eclipse.core.runtime.Assert; import org.eclipse.jface.util.Util; +import org.eclipse.jface.resource.ColorRegistry; +import org.eclipse.jface.resource.JFaceResources; import org.eclipse.swt.SWT; import org.eclipse.swt.graphics.Color; import org.eclipse.swt.graphics.GC; @@ -151,8 +153,8 @@ private void hookListener(final ColumnViewer viewer) { * @return the color or null to use the default */ protected Color getSelectedCellBackgroundColor(ViewerCell cell) { - return removeNonFocusedSelectionInformation ? null - : cell.getItem().getDisplay().getSystemColor(SWT.COLOR_LIST_SELECTION); + ColorRegistry colorRegistry = JFaceResources.getColorRegistry(); + return colorRegistry.get("org.eclipse.ui.workbench.SELECTED_CELL_BACKGROUND"); //$NON-NLS-1$ } /** @@ -164,7 +166,8 @@ protected Color getSelectedCellBackgroundColor(ViewerCell cell) { * @return the color or null to use the default */ protected Color getSelectedCellForegroundColor(ViewerCell cell) { - return null; + ColorRegistry colorRegistry = JFaceResources.getColorRegistry(); + return colorRegistry.get("org.eclipse.ui.workbench.SELECTED_CELL_FOREGROUND"); //$NON-NLS-1$ } /** @@ -178,7 +181,8 @@ protected Color getSelectedCellForegroundColor(ViewerCell cell) { * @since 3.4 */ protected Color getSelectedCellForegroundColorNoFocus(ViewerCell cell) { - return null; + ColorRegistry colorRegistry = JFaceResources.getColorRegistry(); + return colorRegistry.get("org.eclipse.ui.workbench.SELECTED_CELL_FOREGROUND_NO_FOCUS"); //$NON-NLS-1$ } /** @@ -192,7 +196,8 @@ protected Color getSelectedCellForegroundColorNoFocus(ViewerCell cell) { * @since 3.4 */ protected Color getSelectedCellBackgroundColorNoFocus(ViewerCell cell) { - return null; + ColorRegistry colorRegistry = JFaceResources.getColorRegistry(); + return colorRegistry.get("org.eclipse.ui.workbench.SELECTED_CELL_BACKGROUND_NO_FOCUS"); //$NON-NLS-1$ } /** diff --git a/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/OwnerDrawLabelProvider.java b/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/OwnerDrawLabelProvider.java index e1710179f94..2454eda1613 100644 --- a/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/OwnerDrawLabelProvider.java +++ b/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/OwnerDrawLabelProvider.java @@ -17,6 +17,8 @@ import java.util.HashSet; import java.util.Set; +import org.eclipse.jface.resource.ColorRegistry; +import org.eclipse.jface.resource.JFaceResources; import org.eclipse.swt.SWT; import org.eclipse.swt.graphics.Color; import org.eclipse.swt.graphics.Rectangle; @@ -166,20 +168,16 @@ public void update(ViewerCell cell) { } /** - * Handle the erase event. The default implementation colors the background - * of selected areas with {@link SWT#COLOR_LIST_SELECTION} and foregrounds - * with {@link SWT#COLOR_LIST_SELECTION_TEXT}. Note that this - * implementation causes non-native behavior on some platforms. Subclasses - * should override this method and not call the super + * Handle the erase event. The default implementation colors the background of + * selected areas with "org.eclipse.ui.workbench.SELECTED_CELL_BACKGROUND" and + * foregrounds with "org.eclipse.ui.workbench.SELECTED_CELL_FOREGROUND". Note + * that this implementation causes non-native behavior on some platforms. + * Subclasses should override this method and not call the super * implementation. * - * @param event - * the erase event - * @param element - * the model object + * @param event the erase event + * @param element the model object * @see SWT#EraseItem - * @see SWT#COLOR_LIST_SELECTION - * @see SWT#COLOR_LIST_SELECTION_TEXT */ protected void erase(Event event, Object element) { @@ -189,11 +187,16 @@ protected void erase(Event event, Object element) { Color oldForeground = event.gc.getForeground(); Color oldBackground = event.gc.getBackground(); - event.gc.setBackground(event.item.getDisplay().getSystemColor( - SWT.COLOR_LIST_SELECTION)); - event.gc.setForeground(event.item.getDisplay().getSystemColor( - SWT.COLOR_LIST_SELECTION_TEXT)); + ColorRegistry colorRegistry = JFaceResources.getColorRegistry(); + if (event.widget instanceof Control control && control.isFocusControl()) { + event.gc.setBackground(colorRegistry.get("org.eclipse.ui.workbench.SELECTED_CELL_BACKGROUND")); //$NON-NLS-1$ + event.gc.setForeground(colorRegistry.get("org.eclipse.ui.workbench.SELECTED_CELL_FOREGROUND")); //$NON-NLS-1$ + } else { + event.gc.setBackground(colorRegistry.get("org.eclipse.ui.workbench.SELECTED_CELL_BACKGROUND_NO_FOCUS")); //$NON-NLS-1$ + event.gc.setForeground(colorRegistry.get("org.eclipse.ui.workbench.SELECTED_CELL_FOREGROUND_NO_FOCUS")); //$NON-NLS-1$ + } event.gc.fillRectangle(bounds); + /* restore the old GC colors */ event.gc.setForeground(oldForeground); event.gc.setBackground(oldBackground); diff --git a/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/StyledCellLabelProvider.java b/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/StyledCellLabelProvider.java index 4d1872cf13b..1b29e15c69e 100644 --- a/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/StyledCellLabelProvider.java +++ b/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/StyledCellLabelProvider.java @@ -263,6 +263,7 @@ protected void erase(Event event, Object element) { // info has been set by 'update': announce that we paint ourselves event.detail &= ~SWT.FOREGROUND; } + super.erase(event, element); } @Override diff --git a/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TableViewer.java b/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TableViewer.java index 8966573b3f3..6baf6b0e07a 100644 --- a/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TableViewer.java +++ b/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TableViewer.java @@ -20,6 +20,7 @@ package org.eclipse.jface.viewers; import org.eclipse.core.runtime.Assert; +import org.eclipse.jface.viewers.internal.ColumnViewerSelectionColorListener; import org.eclipse.jface.viewers.internal.ExpandableNode; import org.eclipse.pde.api.tools.annotations.NoExtend; import org.eclipse.swt.SWT; @@ -119,6 +120,7 @@ public TableViewer(Composite parent, int style) { public TableViewer(Table table) { this.table = table; hookControl(table); + overwriteSelectionColor(); } @Override @@ -507,4 +509,13 @@ void handleExpandableNodeClicked(Widget w) { } } + /** + * The color of the selected item is drawn by the OS. On some OS the color might + * be not accessible. To fix this issue the background color for selected items + * is drawn in a custom method. + */ + private void overwriteSelectionColor() { + ColumnViewerSelectionColorListener.addListenerToViewer(this); + } + } diff --git a/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TreeViewer.java b/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TreeViewer.java index 55eae5315c0..5f86b75e1ac 100644 --- a/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TreeViewer.java +++ b/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TreeViewer.java @@ -25,6 +25,7 @@ import java.util.List; import org.eclipse.jface.util.Policy; +import org.eclipse.jface.viewers.internal.ColumnViewerSelectionColorListener; import org.eclipse.jface.viewers.internal.ExpandableNode; import org.eclipse.pde.api.tools.annotations.NoExtend; import org.eclipse.swt.SWT; @@ -137,6 +138,7 @@ public TreeViewer(Tree tree) { super(); this.tree = tree; hookControl(tree); + overwriteSelectionColor(); } @Override @@ -1188,4 +1190,13 @@ Object getLastElement(Widget parent) { } return items[length - 1].getData(); } + + /** + * The color of the selected item is drawn by the OS. On some OS the color might + * be not accessible. To fix this issue the background color for selected items + * is drawn in a custom method. + */ + private void overwriteSelectionColor() { + ColumnViewerSelectionColorListener.addListenerToViewer(this); + } } diff --git a/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/internal/ColumnViewerSelectionColorListener.java b/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/internal/ColumnViewerSelectionColorListener.java new file mode 100644 index 00000000000..fbc6e7ea40a --- /dev/null +++ b/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/internal/ColumnViewerSelectionColorListener.java @@ -0,0 +1,83 @@ +/******************************************************************************* + * Copyright (c) 2024 SAP SE. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * SAP SE - initial API and implementation + *******************************************************************************/ +package org.eclipse.jface.viewers.internal; + +import org.eclipse.jface.resource.ColorRegistry; +import org.eclipse.jface.resource.JFaceResources; +import org.eclipse.jface.viewers.FocusCellOwnerDrawHighlighter; +import org.eclipse.jface.viewers.StructuredViewer; +import org.eclipse.swt.SWT; +import org.eclipse.swt.graphics.GC; +import org.eclipse.swt.widgets.Control; +import org.eclipse.swt.widgets.Event; +import org.eclipse.swt.widgets.Listener; +import org.eclipse.swt.widgets.Scrollable; + +/** + * EraseItem event listener that takes care of coloring the selection color of + * column viewers. The coloring is only applied if no other erase item listener + * is registered for the viewer. If other erase item listeners are registers, + * most probably a other customer coloring is applied and should not be + * overwritten. + * + * @see FocusCellOwnerDrawHighlighter + */ +public class ColumnViewerSelectionColorListener implements Listener { + + /** + * Registers an erase item event listener that takes care of coloring the + * selection color of the given viewer. + * + * @param viewer The viewer that should be colored + */ + public static void addListenerToViewer(StructuredViewer viewer) { + viewer.getControl().addListener(SWT.EraseItem, new ColumnViewerSelectionColorListener()); + } + + @Override + public void handleEvent(Event event) { + if ((event.detail & SWT.SELECTED) == 0) { + return; /* item not selected */ + } + + if (event.widget instanceof Control control && !control.isEnabled()) { + return; /* item is disabled, no coloring required */ + } + + Listener[] eraseItemListeners = event.widget.getListeners(SWT.EraseItem); + if (eraseItemListeners.length != 1) { + return; /* other eraseItemListener exists, do not apply coloring */ + } + + GC gc = event.gc; + ColorRegistry colorRegistry = JFaceResources.getColorRegistry(); + if (event.widget instanceof Control control && control.isFocusControl()) { + gc.setBackground(colorRegistry.get("org.eclipse.ui.workbench.SELECTED_CELL_BACKGROUND")); //$NON-NLS-1$ + gc.setForeground(colorRegistry.get("org.eclipse.ui.workbench.SELECTED_CELL_FOREGROUND")); //$NON-NLS-1$ + } else { + gc.setBackground(colorRegistry.get("org.eclipse.ui.workbench.SELECTED_CELL_BACKGROUND_NO_FOCUS")); //$NON-NLS-1$ + gc.setForeground(colorRegistry.get("org.eclipse.ui.workbench.SELECTED_CELL_FOREGROUND_NO_FOCUS")); //$NON-NLS-1$ + } + + int width = event.width; + if (event.widget instanceof Scrollable scrollable) { + width = scrollable.getClientArea().width; + } + + gc.fillRectangle(0, event.y, width, event.height); + + event.detail &= ~SWT.SELECTED; + } + +} diff --git a/bundles/org.eclipse.ui.themes/css/dark/e4-dark_preferencestyle.css b/bundles/org.eclipse.ui.themes/css/dark/e4-dark_preferencestyle.css index b9adbc6bbb2..4fda7024e51 100644 --- a/bundles/org.eclipse.ui.themes/css/dark/e4-dark_preferencestyle.css +++ b/bundles/org.eclipse.ui.themes/css/dark/e4-dark_preferencestyle.css @@ -65,6 +65,8 @@ IEclipsePreferences#org-eclipse-ui-workbench:org-eclipse-ui-themes { /* pseudo a 'org.eclipse.ui.workbench.FORM_HEADING_ERROR_COLOR=255,110,110' 'org.eclipse.ui.workbench.FORM_HEADING_WARNING_COLOR=255,200,0' 'org.eclipse.ui.workbench.FORM_HEADING_INFO_COLOR=170,170,170' + 'org.eclipse.ui.workbench.SELECTED_CELL_FOREGROUND_NO_FOCUS=240,240,240' + 'org.eclipse.ui.workbench.SELECTED_CELL_BACKGROUND_NO_FOCUS=95,95,95' 'ERROR_COLOR=247,68,117' 'HYPERLINK_COLOR=111,197,238' 'INCOMING_COLOR=31,179,235' diff --git a/bundles/org.eclipse.ui.themes/css/e4-dark_win.css b/bundles/org.eclipse.ui.themes/css/e4-dark_win.css index 0b0afa540d0..a61c5268db1 100644 --- a/bundles/org.eclipse.ui.themes/css/e4-dark_win.css +++ b/bundles/org.eclipse.ui.themes/css/e4-dark_win.css @@ -135,3 +135,9 @@ ImageBasedFrame, color: #DDDDDD; } +IEclipsePreferences#org-eclipse-ui-workbench:org-eclipse-ui-themes-win { /* pseudo attribute added to allow contributions without replacing this node, see Bug 466075 */ + preferences: + 'org.eclipse.ui.workbench.SELECTED_CELL_FOREGROUND=255,255,255' + 'org.eclipse.ui.workbench.SELECTED_CELL_BACKGROUND=0,100,255' +} + diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/themes/ColumnViewerSelectionColorFactory.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/themes/ColumnViewerSelectionColorFactory.java new file mode 100644 index 00000000000..a2e852d3458 --- /dev/null +++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/themes/ColumnViewerSelectionColorFactory.java @@ -0,0 +1,63 @@ +/******************************************************************************* + * Copyright (c) 2024 SAP SE. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * SAP SE - initial API and implementation + *******************************************************************************/ +package org.eclipse.ui.internal.themes; + +import java.util.Hashtable; +import org.eclipse.core.runtime.IConfigurationElement; +import org.eclipse.core.runtime.IExecutableExtension; +import org.eclipse.swt.SWT; +import org.eclipse.swt.graphics.RGB; +import org.eclipse.swt.widgets.Display; +import org.eclipse.ui.themes.IColorFactory; + +/** + * The factory to provide the default colors for viewer selections. The default + * color is based on the system color for the title, which is the actual + * highlight color. The color can be overwritten via the color preferences. + * + * @since 3.5 + * + */ +public class ColumnViewerSelectionColorFactory implements IColorFactory, IExecutableExtension { + + private String color = null; + + @Override + public RGB createColor() { + if ("SELECTED_CELL_BACKGROUND".equals(color)) { //$NON-NLS-1$ + return Display.getDefault().getSystemColor(SWT.COLOR_TITLE_BACKGROUND).getRGB(); + + } else if ("SELECTED_CELL_FOREGROUND".equals(color)) { //$NON-NLS-1$ + // Selection foreground should be white if not changed by the user + return Display.getDefault().getSystemColor(SWT.COLOR_WHITE).getRGB(); + + } else if ("SELECTED_CELL_BACKGROUND_NO_FOCUS".equals(color)) { //$NON-NLS-1$ + return Display.getDefault().getSystemColor(SWT.COLOR_TITLE_INACTIVE_BACKGROUND).getRGB(); + + } else if ("SELECTED_CELL_FOREGROUND_NO_FOCUS".equals(color)) { //$NON-NLS-1$ + return Display.getDefault().getSystemColor(SWT.COLOR_TITLE_INACTIVE_FOREGROUND).getRGB(); + + } else { + return null; + } + } + + @Override + public void setInitializationData(IConfigurationElement config, String propertyName, Object data) { + if (data instanceof Hashtable table) { + this.color = (String) table.get("color"); //$NON-NLS-1$ + } + } + +} \ No newline at end of file diff --git a/bundles/org.eclipse.ui/plugin.properties b/bundles/org.eclipse.ui/plugin.properties index 525c2caece2..af2b0558d89 100644 --- a/bundles/org.eclipse.ui/plugin.properties +++ b/bundles/org.eclipse.ui/plugin.properties @@ -374,6 +374,15 @@ Color.showKeysForegroundDesc=Color for foreground of the control to visualize pr Color.showKeysBackground=Keys background color Color.showKeysBackgroundDesc=Color for background of the control to visualize pressed keys +Color.selectedCellForeground=Selection foreground color for cell viewer +Color.selectedCellForegroundDesc=The selection foreground color of a cell viewer +Color.selectedCellBackground=Selection background color for cell viewer +Color.selectedCellBackgroundDesc=The selection background color of a cell viewer +Color.selectedCellForegroundNoFocus=Selection foreground color for cell viewer without focus +Color.selectedCellForegroundNoFocusDesc=The selection foreground color of a cell viewer without focus +Color.selectedCellBackgroundNoFocus=Selection background color for cell viewer without focus +Color.selectedCellBackgroundNoFocusDesc=The selection background color of a cell viewer without focus + ThemeName.SystemDefault = Reduced Palette HighContrast.ThemeDescription = A theme that takes all of its values from the system settings. diff --git a/bundles/org.eclipse.ui/plugin.xml b/bundles/org.eclipse.ui/plugin.xml index f4523fefbd8..89153764ace 100644 --- a/bundles/org.eclipse.ui/plugin.xml +++ b/bundles/org.eclipse.ui/plugin.xml @@ -2054,6 +2054,74 @@ %Color.showKeysBackgroundDesc + + + %Color.selectedCellForegroundDesc + + + + + + + + + %Color.selectedCellBackgroundDesc + + + + + + + + + %Color.selectedCellForegroundNoFocusDesc + + + + + + + + + %Color.selectedCellBackgroundNoFocusDesc + + + + + + diff --git a/tests/org.eclipse.jface.text.tests/META-INF/MANIFEST.MF b/tests/org.eclipse.jface.text.tests/META-INF/MANIFEST.MF index fca4c84b39c..eca971f2fee 100644 --- a/tests/org.eclipse.jface.text.tests/META-INF/MANIFEST.MF +++ b/tests/org.eclipse.jface.text.tests/META-INF/MANIFEST.MF @@ -22,7 +22,8 @@ Require-Bundle: org.eclipse.text.tests;bundle-version="[3.5.0,4.0.0)", org.eclipse.core.runtime;bundle-version="[3.29.0,4.0.0)", org.eclipse.ui.workbench.texteditor, - org.eclipse.test.performance;bundle-version="3.13.0" + org.eclipse.test.performance;bundle-version="3.13.0", + org.mockito.mockito-core;bundle-version="5.12.0" Bundle-RequiredExecutionEnvironment: JavaSE-17 Eclipse-BundleShape: dir Automatic-Module-Name: org.eclipse.jface.text.tests diff --git a/tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/internal/text/TableOwnerDrawSupportTest.java b/tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/internal/text/TableOwnerDrawSupportTest.java new file mode 100644 index 00000000000..558c5c9aaeb --- /dev/null +++ b/tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/internal/text/TableOwnerDrawSupportTest.java @@ -0,0 +1,93 @@ +package org.eclipse.jface.internal.text; + +import static org.mockito.Mockito.doReturn; + +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; + +import org.eclipse.swt.SWT; +import org.eclipse.swt.graphics.Color; +import org.eclipse.swt.graphics.GC; +import org.eclipse.swt.graphics.RGB; +import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Event; +import org.eclipse.swt.widgets.Shell; +import org.eclipse.swt.widgets.Table; +import org.eclipse.swt.widgets.TableItem; + +import org.eclipse.jface.resource.ColorRegistry; +import org.eclipse.jface.resource.JFaceResources; + +public class TableOwnerDrawSupportTest { + + private static final int EVENT_Y= 2; + + private static final int EVENT_HEIGHT= 3; + + private Table table; + + private Event event; + + private RGB selectedRowBackgroundColor= new RGB(0, 0, 0); + + private RGB selectedRowForegroundColor= new RGB(1, 1, 1); + + private RGB selectedRowBackgroundColorNoFocus= new RGB(2, 2, 2); + + private RGB selectedRowForegroundColorNoFocus= new RGB(3, 3, 3); + + private GC gc; + + @Before + public void setup() { + Shell shell= new Shell(Display.getDefault()); + Table originalTable= new Table(shell, SWT.SINGLE); + //Spy in order to overwrite isFocusControl() + table= Mockito.spy(originalTable); + + TableItem tableItem= Mockito.mock(TableItem.class); + Mockito.when(tableItem.getParent()).thenReturn(table); + + gc= Mockito.mock(GC.class); + event= new Event(); + event.type= SWT.PaintItem; + event.gc= gc; + event.detail= SWT.SELECTED; + event.item= tableItem; + event.y= EVENT_Y; + event.height= EVENT_HEIGHT; + + Display.getDefault(); + ColorRegistry colorRegistry= JFaceResources.getColorRegistry(); + colorRegistry.put("org.eclipse.ui.workbench.SELECTED_CELL_BACKGROUND", new RGB(0, 0, 0)); + colorRegistry.put("org.eclipse.ui.workbench.SELECTED_CELL_FOREGROUND", new RGB(1, 1, 1)); + colorRegistry.put("org.eclipse.ui.workbench.SELECTED_CELL_BACKGROUND_NO_FOCUS", new RGB(2, 2, 2)); + colorRegistry.put("org.eclipse.ui.workbench.SELECTED_CELL_FOREGROUND_NO_FOCUS", new RGB(3, 3, 3)); + + TableOwnerDrawSupport.install(table); + } + + @Test + public void testPaintSelectionFocus() { + doReturn(true).when(table).isFocusControl(); + + table.notifyListeners(SWT.PaintItem, event); + + Mockito.verify(gc).setBackground(new Color(selectedRowBackgroundColor)); + Mockito.verify(gc).setForeground(new Color(selectedRowForegroundColor)); + Mockito.verify(gc).fillRectangle(0, EVENT_Y, table.getBounds().width, EVENT_HEIGHT); + } + + @Test + public void testPaintSelectionNoFocus() { + doReturn(false).when(table).isFocusControl(); + + table.notifyListeners(SWT.PaintItem, event); + + Mockito.verify(gc).setBackground(new Color(selectedRowBackgroundColorNoFocus)); + Mockito.verify(gc).setForeground(new Color(selectedRowForegroundColorNoFocus)); + Mockito.verify(gc).fillRectangle(0, EVENT_Y, table.getBounds().width, EVENT_HEIGHT); + } + +} diff --git a/tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/internal/text/contentassist/CompletionTableDrawSupportTest.java b/tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/internal/text/contentassist/CompletionTableDrawSupportTest.java new file mode 100644 index 00000000000..b89a9833bfc --- /dev/null +++ b/tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/internal/text/contentassist/CompletionTableDrawSupportTest.java @@ -0,0 +1,92 @@ +package org.eclipse.jface.internal.text.contentassist; + +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.collection.IsArrayWithSize.arrayWithSize; +import static org.hamcrest.collection.IsArrayWithSize.emptyArray; +import static org.junit.Assert.assertEquals; + +import org.junit.Test; +import org.mockito.Mockito; + +import org.eclipse.swt.SWT; +import org.eclipse.swt.custom.StyleRange; +import org.eclipse.swt.graphics.Color; +import org.eclipse.swt.graphics.GC; +import org.eclipse.swt.graphics.RGB; +import org.eclipse.swt.widgets.Event; +import org.eclipse.swt.widgets.Shell; +import org.eclipse.swt.widgets.Table; +import org.eclipse.swt.widgets.TableItem; + +import org.eclipse.jface.internal.text.TableOwnerDrawSupport; +import org.eclipse.jface.resource.ColorRegistry; +import org.eclipse.jface.resource.JFaceResources; + +public class CompletionTableDrawSupportTest { + + @Test + public void testInstall() { + Shell shell= new Shell(); + Table table= new Table(shell, SWT.NONE); + + assertThat(table.getListeners(SWT.MeasureItem), emptyArray()); + assertThat(table.getListeners(SWT.EraseItem), emptyArray()); + assertThat(table.getListeners(SWT.PaintItem), emptyArray()); + + CompletionTableDrawSupport.install(table); + + assertThat(table.getListeners(SWT.MeasureItem), arrayWithSize(1)); + assertThat(table.getListeners(SWT.MeasureItem)[0], instanceOf(TableOwnerDrawSupport.class)); + assertThat(table.getListeners(SWT.EraseItem), arrayWithSize(1)); + assertThat(table.getListeners(SWT.EraseItem)[0], instanceOf(TableOwnerDrawSupport.class)); + assertThat(table.getListeners(SWT.PaintItem), arrayWithSize(1)); + assertThat(table.getListeners(SWT.PaintItem)[0], instanceOf(TableOwnerDrawSupport.class)); + } + + @Test + public void testStoreStyleRanges() { + Shell shell= new Shell(); + Table table= new Table(shell, SWT.NONE); + TableItem tableItem= new TableItem(table, SWT.NONE); + StyleRange[] ranges= new StyleRange[] {}; + + CompletionTableDrawSupport.storeStyleRanges(tableItem, 2, ranges); + + assertEquals(ranges, tableItem.getData("styled_ranges2")); + } + + @Test + public void testPaintNonFocusSelectionInFocusColors() { + int EVENT_Y= 2; + int EVENT_HEIGHT= 3; + + Shell shell= new Shell(); + Table table= new Table(shell, SWT.NONE); + TableItem tableItem= new TableItem(table, SWT.NONE); + + ColorRegistry colorRegistry= JFaceResources.getColorRegistry(); + colorRegistry.put("org.eclipse.ui.workbench.SELECTED_CELL_BACKGROUND", new RGB(0, 0, 0)); + colorRegistry.put("org.eclipse.ui.workbench.SELECTED_CELL_FOREGROUND", new RGB(1, 1, 1)); + colorRegistry.put("org.eclipse.ui.workbench.SELECTED_CELL_BACKGROUND_NO_FOCUS", new RGB(2, 2, 2)); + colorRegistry.put("org.eclipse.ui.workbench.SELECTED_CELL_FOREGROUND_NO_FOCUS", new RGB(3, 3, 3)); + + GC gc= Mockito.mock(GC.class); + Event event= new Event(); + event.type= SWT.PaintItem; + event.gc= gc; + event.detail= SWT.SELECTED; + event.item= tableItem; + event.y= EVENT_Y; + event.height= EVENT_HEIGHT; + + CompletionTableDrawSupport.install(table); + + table.notifyListeners(SWT.PaintItem, event); + + Mockito.verify(gc).setBackground(new Color(new RGB(0, 0, 0))); + Mockito.verify(gc).setForeground(new Color(new RGB(1, 1, 1))); + Mockito.verify(gc).fillRectangle(0, EVENT_Y, table.getBounds().width, EVENT_HEIGHT); + } + +} diff --git a/tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/JFaceTextTestSuite.java b/tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/JFaceTextTestSuite.java index 1b190e48845..cc939d99d56 100644 --- a/tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/JFaceTextTestSuite.java +++ b/tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/JFaceTextTestSuite.java @@ -17,6 +17,9 @@ import org.junit.runners.Suite; import org.junit.runners.Suite.SuiteClasses; +import org.eclipse.jface.internal.text.TableOwnerDrawSupportTest; +import org.eclipse.jface.internal.text.contentassist.CompletionTableDrawSupportTest; + import org.eclipse.jface.text.tests.codemining.CodeMiningProjectionViewerTest; import org.eclipse.jface.text.tests.codemining.CodeMiningTest; import org.eclipse.jface.text.tests.contentassist.AsyncContentAssistTest; @@ -80,7 +83,10 @@ DefaultTextDoubleClickStrategyTest.class, MultiSelectionTest.class, FindReplaceDocumentAdapterContentProposalProviderTest.class, - ProjectionViewerTest.class + ProjectionViewerTest.class, + + TableOwnerDrawSupportTest.class, + CompletionTableDrawSupportTest.class }) public class JFaceTextTestSuite { // see @SuiteClasses