Skip to content

Commit

Permalink
Highlighting problem when using the dark theme on Windows
Browse files Browse the repository at this point in the history
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 eclipse-platform/eclipse.platform.swt#811
Fixes #1688
  • Loading branch information
Christopher-Hermann authored and fedejeanne committed Jun 11, 2024
1 parent 934ccc6 commit cbda3bc
Show file tree
Hide file tree
Showing 20 changed files with 624 additions and 34 deletions.
1 change: 1 addition & 0 deletions bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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 <code>null</code> 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 <code>null</code> 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 <b>no</b> input focus
*
* @return the color or <code>null</code> 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
* <b>no</b> input focus
*
* @return the color or <code>null</code> 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() {
Expand Down
Original file line number Diff line number Diff line change
@@ -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 <code>null</code> 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();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -613,7 +613,7 @@ void createProposalSelector() {

fIsColoredLabelsSupportEnabled= fContentAssistant.isColoredLabelsSupportEnabled();
if (fIsColoredLabelsSupportEnabled)
TableOwnerDrawSupport.install(fProposalTable);
CompletionTableDrawSupport.install(fProposalTable);

fProposalTable.setLocation(0, 0);
if (fAdditionalInfoController != null)
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -151,8 +153,8 @@ private void hookListener(final ColumnViewer viewer) {
* @return the color or <code>null</code> 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$
}

/**
Expand All @@ -164,7 +166,8 @@ protected Color getSelectedCellBackgroundColor(ViewerCell cell) {
* @return the color or <code>null</code> 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$
}

/**
Expand All @@ -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$
}

/**
Expand All @@ -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$
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 <b>not</b> 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 <b>not</b> 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) {

Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -119,6 +120,7 @@ public TableViewer(Composite parent, int style) {
public TableViewer(Table table) {
this.table = table;
hookControl(table);
overwriteSelectionColor();
}

@Override
Expand Down Expand Up @@ -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);
}

}
Loading

0 comments on commit cbda3bc

Please sign in to comment.