From c0fa2b5333db8bb5f2b7202a3fbe2d3e423d4b87 Mon Sep 17 00:00:00 2001 From: Christopher Hermann Date: Fri, 14 Jun 2024 10:41:54 +0200 Subject: [PATCH] =?UTF-8?q?[Sticky=20Scrolling]=C2=A0Fix=20separator=20col?= =?UTF-8?q?or=20on=20windows?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- bundles/org.eclipse.ui.editors/plugin.xml | 6 +++++ .../EditorsPluginPreferenceInitializer.java | 4 +++ .../texteditor/ITextEditorThemeConstants.java | 6 +++++ .../stickyscroll/StickyScrollingControl.java | 26 +++++++++---------- .../StickyScrollingControlSettings.java | 2 ++ .../stickyscroll/StickyScrollingHandler.java | 17 ++++++++++-- .../css/dark/e4-dark_preferencestyle.css | 1 + .../StickyScrollingControlTest.java | 20 +++++++++++--- .../StickyScrollingHandlerTest.java | 4 ++- 9 files changed, 65 insertions(+), 21 deletions(-) diff --git a/bundles/org.eclipse.ui.editors/plugin.xml b/bundles/org.eclipse.ui.editors/plugin.xml index 29965b59fd2..e69aca3a288 100644 --- a/bundles/org.eclipse.ui.editors/plugin.xml +++ b/bundles/org.eclipse.ui.editors/plugin.xml @@ -1113,6 +1113,12 @@ label="%dummy" value="0,0,0"> + + diff --git a/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/editors/text/EditorsPluginPreferenceInitializer.java b/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/editors/text/EditorsPluginPreferenceInitializer.java index 859b66fcea3..6000aaba7d4 100644 --- a/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/editors/text/EditorsPluginPreferenceInitializer.java +++ b/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/editors/text/EditorsPluginPreferenceInitializer.java @@ -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); diff --git a/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/texteditor/ITextEditorThemeConstants.java b/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/texteditor/ITextEditorThemeConstants.java index b9da5f192b5..9f8cc713039 100644 --- a/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/texteditor/ITextEditorThemeConstants.java +++ b/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/texteditor/ITextEditorThemeConstants.java @@ -64,4 +64,10 @@ 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 print margin (value + * "org.eclipse.ui.editors.printMarginColor"). + */ + public final static String STICKY_LINES_SEPARATOR_COLOR= "org.eclipse.ui.editors.stickyLinesSeparatorColor"; //$NON-NLS-1$ + } diff --git a/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingControl.java b/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingControl.java index 1e328fcae49..5050e240113 100644 --- a/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingControl.java +++ b/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingControl.java @@ -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; @@ -77,6 +76,8 @@ */ public class StickyScrollingControl { + private static final String DISABLE_CSS= "org.eclipse.e4.ui.css.disabled"; //$NON-NLS-1$ + private List stickyLines; private ISourceViewer sourceViewer; @@ -97,9 +98,7 @@ public class StickyScrollingControl { private StickyScollingCaretListener caretListener; - private Label bottomSeparator; - - private final static int BOTTOM_SEPARATOR_SPACING= 2; + private Composite bottomSeparator; private StickyScrollingHandler stickyScrollingHandler; @@ -137,6 +136,7 @@ public void applySettings(StickyScrollingControlSettings newSettings) { stickyLineNumber.setBackground(newSettings.stickyLineBackgroundColor()); stickyLineText.setBackground(newSettings.stickyLineBackgroundColor()); + bottomSeparator.setBackground(settings.stickyLinesSeparatorColor()); updateStickyScrollingControls(); } @@ -176,13 +176,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()); stickyLinesCanvas.moveAbove(null); } @@ -274,10 +272,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. */ @@ -335,7 +333,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; @@ -382,7 +380,7 @@ private int getNumberStickyLines() { /** * Add several listeners to the source viewer.
- * + * * textPresentationListener in order to style the sticky lines when the source viewer styling * has changed.
*
diff --git a/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingControlSettings.java b/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingControlSettings.java index d0f4ff7ccde..a278024154c 100644 --- a/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingControlSettings.java +++ b/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingControlSettings.java @@ -24,6 +24,7 @@ * @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( @@ -31,5 +32,6 @@ public record StickyScrollingControlSettings( Color lineNumberColor, Color stickyLineHoverColor, Color stickyLineBackgroundColor, + Color stickyLinesSeparatorColor, boolean showLineNumbers) { } diff --git a/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingHandler.java b/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingHandler.java index e77ab3c8816..05b8813af39 100644 --- a/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingHandler.java +++ b/bundles/org.eclipse.ui.editors/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingHandler.java @@ -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; @@ -23,6 +24,7 @@ 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; @@ -30,9 +32,12 @@ 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. @@ -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); @@ -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 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..e08713009f9 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 @@ -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 */ diff --git a/tests/org.eclipse.ui.editors.tests/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingControlTest.java b/tests/org.eclipse.ui.editors.tests/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingControlTest.java index 0911f550936..898a9cbd06f 100644 --- a/tests/org.eclipse.ui.editors.tests/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingControlTest.java +++ b/tests/org.eclipse.ui.editors.tests/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingControlTest.java @@ -52,6 +52,7 @@ public class StickyScrollingControlTest { private Color lineNumberColor; private Color hoverColor; private Color backgroundColor; + private Color separatorColor; private StickyScrollingControl stickyScrollingControl; private CompositeRuler ruler; @@ -66,8 +67,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); } @@ -104,6 +106,9 @@ public void testCorrectColorsApplied() { StyledText stickyLineText = getStickyLineText(); assertEquals(backgroundColor, stickyLineText.getBackground()); + + Composite stickyLineSeparator = getStickyLineSeparator(); + assertEquals(separatorColor, stickyLineSeparator.getBackground()); } @Test @@ -112,7 +117,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(); @@ -142,7 +147,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(); @@ -251,7 +256,9 @@ public void testHorizontalScrollingIsDispatched() { 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; } } @@ -272,4 +279,9 @@ private StyledText getStickyLineText() { return (StyledText) canvas.getChildren()[1]; } + private Composite getStickyLineSeparator() { + Canvas canvas = getStickyControlCanvas(shell); + return (Composite) canvas.getChildren()[2]; + } + } diff --git a/tests/org.eclipse.ui.editors.tests/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingHandlerTest.java b/tests/org.eclipse.ui.editors.tests/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingHandlerTest.java index e6fab2c3978..4a420ec6add 100644 --- a/tests/org.eclipse.ui.editors.tests/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingHandlerTest.java +++ b/tests/org.eclipse.ui.editors.tests/src/org/eclipse/ui/internal/texteditor/stickyscroll/StickyScrollingHandlerTest.java @@ -172,7 +172,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; } }