Skip to content

Commit

Permalink
[Sticky Scrolling] Fix separator color on windows
Browse files Browse the repository at this point in the history
The separator in the sticky lines controls are drawn in a bad color on windows dark theme. With this change, the separator color is aligned with the theme.
  • Loading branch information
Christopher-Hermann committed Jul 12, 2024
1 parent 4888a8c commit fc4da2d
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 21 deletions.
6 changes: 6 additions & 0 deletions bundles/org.eclipse.ui.editors/plugin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1113,6 +1113,12 @@
label="%dummy"
value="0,0,0">
</colorDefinition>
<colorDefinition
id="org.eclipse.ui.editors.stickyLinesSeparatorColor"
isEditable="false"
label="test"
value="232,232,232">
</colorDefinition>

<theme
id="org.eclipse.ui.ide.systemDefault">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ public static void setThemeBasedPreferences(IPreferenceStore store, boolean fire
AbstractDecoratedTextEditorPreferenceConstants.EDITOR_PRINT_MARGIN_COLOR,
findRGB(registry,ITextEditorThemeConstants.PRINT_MARGIN_COLOR, new RGB(176, 180 , 185)), fireEvent);

setDefault(store,
ITextEditorThemeConstants.STICKY_LINES_SEPARATOR_COLOR,
findRGB(registry, ITextEditorThemeConstants.STICKY_LINES_SEPARATOR_COLOR, new RGB(232, 232, 232)), fireEvent);

setDefault(store,
AbstractDecoratedTextEditorPreferenceConstants.EDITOR_LINE_NUMBER_RULER_COLOR,
findRGB(registry, ITextEditorThemeConstants.LINE_NUMBER_RULER_COLOR, new RGB(120, 120, 120)), fireEvent);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,9 @@ public interface ITextEditorThemeConstants {
*/
public static final String PREFERENCE_COLOR_FOREGROUND= "org.eclipse.ui.editors.foregroundColor"; //$NON-NLS-1$

/**
* Theme constant for the color used to render the sticky lines separator.
*/
public final static String STICKY_LINES_SEPARATOR_COLOR= "org.eclipse.ui.editors.stickyLinesSeparatorColor"; //$NON-NLS-1$

}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import org.eclipse.swt.widgets.Composite;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Event;
import org.eclipse.swt.widgets.Label;
import org.eclipse.swt.widgets.Listener;
import org.eclipse.swt.widgets.ScrollBar;

Expand Down Expand Up @@ -86,6 +85,8 @@ public class StickyScrollingControl {
*/
private final static int MIN_VISIBLE_EDITOR_LINES_THRESHOLD= 3;

private static final String DISABLE_CSS= "org.eclipse.e4.ui.css.disabled"; //$NON-NLS-1$

private List<StickyLine> stickyLines;

private ISourceViewer sourceViewer;
Expand All @@ -106,9 +107,7 @@ public class StickyScrollingControl {

private StickyScollingCaretListener caretListener;

private Label bottomSeparator;

private final static int BOTTOM_SEPARATOR_SPACING= 2;
private Composite bottomSeparator;

private StickyScrollingHandler stickyScrollingHandler;

Expand Down Expand Up @@ -148,6 +147,7 @@ public void applySettings(StickyScrollingControlSettings newSettings) {

stickyLineNumber.setBackground(newSettings.stickyLineBackgroundColor());
stickyLineText.setBackground(newSettings.stickyLineBackgroundColor());
bottomSeparator.setBackground(settings.stickyLinesSeparatorColor());

updateStickyScrollingControls();
}
Expand Down Expand Up @@ -187,13 +187,11 @@ private void createControls() {
stickyLineText.setEnabled(false);
stickyLineText.setBackground(settings.stickyLineBackgroundColor());

bottomSeparator= new Label(stickyLinesCanvas, SWT.SEPARATOR | SWT.SHADOW_OUT | SWT.HORIZONTAL);
GridDataFactory.fillDefaults().grab(true, false).span(2, 1).applyTo(bottomSeparator);
bottomSeparator.setEnabled(false);

bottomSeparator= new Label(stickyLinesCanvas, SWT.SEPARATOR | SWT.SHADOW_OUT | SWT.HORIZONTAL);
GridDataFactory.fillDefaults().grab(true, false).indent(0, BOTTOM_SEPARATOR_SPACING).span(2, 1).applyTo(bottomSeparator);
bottomSeparator= new Composite(stickyLinesCanvas, SWT.NONE);
GridDataFactory.fillDefaults().hint(0, 3).grab(true, false).span(2, 1).applyTo(bottomSeparator);
bottomSeparator.setEnabled(false);
bottomSeparator.setData(DISABLE_CSS, Boolean.TRUE);
bottomSeparator.setBackground(settings.stickyLinesSeparatorColor());

layoutLineNumbers();
limitVisibleStickyLinesToTextWidgetHeight(sourceViewer.getTextWidget());
Expand Down Expand Up @@ -288,10 +286,10 @@ private void layoutStickyLines() {

/**
* The line numbers layout is calculated based on the given {@link #verticalRuler}.
*
*
* If the vertical ruler is an instance of {@link CompositeRuler}, it is tried to align the
* layout with the layout of the {@link LineNumberColumn}.
*
*
* If the vertical ruler is from another instance, the lines number are align in the center of
* the vertical ruler space.
*/
Expand Down Expand Up @@ -349,7 +347,7 @@ private void calculateAndSetStickyLinesCanvasBounds() {
int numberStickyLines= getNumberStickyLines();
int lineHeight= stickyLineText.getLineHeight() * numberStickyLines;
int spacingHeight= stickyLineText.getLineSpacing() * (numberStickyLines - 1);
int separatorHeight= bottomSeparator.getBounds().height * 2 + BOTTOM_SEPARATOR_SPACING;
int separatorHeight= bottomSeparator.getBounds().height;

int rulerWidth= verticalRuler != null ? verticalRuler.getWidth() : 0;
int textWidth= textWidget.getClientArea().width + 1;
Expand Down Expand Up @@ -398,7 +396,7 @@ private int getNumberStickyLines() {

/**
* Add several listeners to the source viewer.<br>
*
*
* textPresentationListener in order to style the sticky lines when the source viewer styling
* has changed.<br>
* <br>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@
* @param lineNumberColor The color used to display line numbers
* @param stickyLineHoverColor The color used to display sticky lines while hovering over them
* @param stickyLineBackgroundColor The color used to display sticky lines back ground color
* @param stickyLinesSeparatorColor The color used to display sticky lines separator color
* @param showLineNumbers Specifies if line numbers should be showed for sticky scrolling
*/
public record StickyScrollingControlSettings(
int maxCountStickyLines,
Color lineNumberColor,
Color stickyLineHoverColor,
Color stickyLineBackgroundColor,
Color stickyLinesSeparatorColor,
boolean showLineNumbers) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*******************************************************************************/
package org.eclipse.ui.internal.texteditor.stickyscroll;

import static org.eclipse.ui.internal.texteditor.ITextEditorThemeConstants.STICKY_LINES_SEPARATOR_COLOR;
import static org.eclipse.ui.texteditor.AbstractDecoratedTextEditorPreferenceConstants.EDITOR_CURRENT_LINE_COLOR;
import static org.eclipse.ui.texteditor.AbstractDecoratedTextEditorPreferenceConstants.EDITOR_LINE_NUMBER_RULER;
import static org.eclipse.ui.texteditor.AbstractDecoratedTextEditorPreferenceConstants.EDITOR_LINE_NUMBER_RULER_COLOR;
Expand All @@ -23,16 +24,20 @@
import java.util.List;

import org.eclipse.swt.graphics.Color;
import org.eclipse.swt.graphics.RGB;

import org.eclipse.jface.preference.IPreferenceStore;
import org.eclipse.jface.preference.PreferenceConverter;
import org.eclipse.jface.util.IPropertyChangeListener;
import org.eclipse.jface.util.Throttler;

import org.eclipse.jface.text.IViewportListener;
import org.eclipse.jface.text.source.ISharedTextColors;
import org.eclipse.jface.text.source.ISourceViewer;
import org.eclipse.jface.text.source.IVerticalRuler;

import org.eclipse.ui.internal.editors.text.EditorsPlugin;

/**
* A sticky scrolling handler that retrieves stick lines from the {@link StickyLinesProvider} and
* shows them in a {@link StickyScrollingControl} on top of the given source viewer.
Expand Down Expand Up @@ -94,7 +99,8 @@ private StickyScrollingControlSettings loadAndListenForProperties(IPreferenceSto
preferenceStore= store;
propertyChangeListener= e -> {
if (e.getProperty().equals(EDITOR_TAB_WIDTH) || e.getProperty().equals(EDITOR_STICKY_SCROLLING_MAXIMUM_COUNT)
|| e.getProperty().equals(EDITOR_CURRENT_LINE_COLOR) || e.getProperty().equals(EDITOR_LINE_NUMBER_RULER)) {
|| e.getProperty().equals(EDITOR_CURRENT_LINE_COLOR) || e.getProperty().equals(EDITOR_LINE_NUMBER_RULER)
|| e.getProperty().equals(STICKY_LINES_SEPARATOR_COLOR)) {
if (stickyScrollingControl != null && !sourceViewer.getTextWidget().isDisposed()) {
StickyScrollingControlSettings settings= loadSettings(preferenceStore);
stickyScrollingControl.applySettings(settings);
Expand All @@ -121,8 +127,15 @@ private StickyScrollingControlSettings loadSettings(IPreferenceStore store) {

boolean showLineNumbers= store.getBoolean(EDITOR_LINE_NUMBER_RULER);

Color stickyLineSeparatorColor= null;
if (EditorsPlugin.getDefault() != null) {
RGB rgb= PreferenceConverter.getColor(store, STICKY_LINES_SEPARATOR_COLOR);
ISharedTextColors sharedTextColors= EditorsPlugin.getDefault().getSharedTextColors();
stickyLineSeparatorColor= sharedTextColors.getColor(rgb);
}

return new StickyScrollingControlSettings(stickyScrollingMaxCount,
lineNumberColor, stickyLineHoverColor, stickyLineBackgroundColor, showLineNumbers);
lineNumberColor, stickyLineHoverColor, stickyLineBackgroundColor, stickyLineSeparatorColor, showLineNumbers);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ IEclipsePreferences#org-eclipse-ui-editors:org-eclipse-ui-themes { /* pseudo att
'secondaryIPColor=90,90,90'
'spellingIndicationColor=253,170,211'
'writeOccurrenceIndicationColor=27,98,145'
'org.eclipse.ui.editors.stickyLinesSeparatorColor=81,86,88'
}

IEclipsePreferences#org-eclipse-ui-workbench:org-eclipse-ui-themes { /* pseudo attribute added to allow contributions without replacing this node, see Bug 466075 */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public class StickyScrollingControlTest {
private Color lineNumberColor;
private Color hoverColor;
private Color backgroundColor;
private Color separatorColor;
private StickyScrollingControl stickyScrollingControl;
private CompositeRuler ruler;

Expand All @@ -67,8 +68,9 @@ public void setup() {
lineNumberColor = new Color(0, 0, 0);
hoverColor = new Color(1, 1, 1);
backgroundColor = new Color(2, 2, 2);
separatorColor = new Color(3, 3, 3);
StickyScrollingControlSettings settings = new StickyScrollingControlSettings(5, lineNumberColor, hoverColor,
backgroundColor, true);
backgroundColor, separatorColor, true);
stickyScrollingControl = new StickyScrollingControl(sourceViewer, ruler, settings, null);
}

Expand Down Expand Up @@ -105,6 +107,9 @@ public void testCorrectColorsApplied() {

StyledText stickyLineText = getStickyLineText();
assertEquals(backgroundColor, stickyLineText.getBackground());

Composite stickyLineSeparator = getStickyLineSeparator();
assertEquals(separatorColor, stickyLineSeparator.getBackground());
}

@Test
Expand All @@ -113,7 +118,7 @@ public void testLimitStickyLinesCount() {
stickyScrollingControl.setStickyLines(stickyLines);

StickyScrollingControlSettings settings = new StickyScrollingControlSettings(1, lineNumberColor, hoverColor,
backgroundColor, true);
backgroundColor, separatorColor, true);
stickyScrollingControl.applySettings(settings);

StyledText stickyLineNumber = getStickyLineNumber();
Expand Down Expand Up @@ -141,7 +146,7 @@ public void testCopyStyleRanges() {
public void testWithoutVeticalRuler() {
sourceViewer = new SourceViewer(shell, null, SWT.None);
StickyScrollingControlSettings settings = new StickyScrollingControlSettings(5, lineNumberColor, hoverColor,
backgroundColor, true);
backgroundColor, separatorColor, true);
stickyScrollingControl = new StickyScrollingControl(sourceViewer, settings);

StyledText stickyLineNumber = getStickyLineNumber();
Expand Down Expand Up @@ -265,7 +270,9 @@ public void limitStickyLinesToTextWidgetHeight() {
private Canvas getStickyControlCanvas(Composite composite) {
for (Control control : composite.getChildren()) {
if (control instanceof Canvas canvas) {
if (canvas.getChildren().length == 4) {
if (canvas.getChildren().length == 3 && canvas.getChildren()[0] instanceof StyledText
&& canvas.getChildren()[1] instanceof StyledText
&& canvas.getChildren()[2] instanceof Composite) {
return canvas;
}
}
Expand All @@ -286,4 +293,9 @@ private StyledText getStickyLineText() {
return (StyledText) canvas.getChildren()[1];
}

private Composite getStickyLineSeparator() {
Canvas canvas = getStickyControlCanvas(shell);
return (Composite) canvas.getChildren()[2];
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,9 @@ private StyledText getStickyLineText() {
private Canvas getStickyControlCanvas(Composite composite) {
for (Control control : composite.getChildren()) {
if (control instanceof Canvas canvas) {
if (canvas.getChildren().length == 4) {
if (canvas.getChildren().length == 3 && canvas.getChildren()[0] instanceof StyledText
&& canvas.getChildren()[1] instanceof StyledText
&& canvas.getChildren()[2] instanceof Composite) {
return canvas;
}
}
Expand Down

0 comments on commit fc4da2d

Please sign in to comment.