From fc4da2d487471203f20763817b89240459ba3bfb 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 | 5 ++++ .../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, 64 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..aba586c66ef 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,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$ + } 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 07452e4d2ee..59078bc5a88 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; @@ -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 stickyLines; private ISourceViewer sourceViewer; @@ -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; @@ -148,6 +147,7 @@ public void applySettings(StickyScrollingControlSettings newSettings) { stickyLineNumber.setBackground(newSettings.stickyLineBackgroundColor()); stickyLineText.setBackground(newSettings.stickyLineBackgroundColor()); + bottomSeparator.setBackground(settings.stickyLinesSeparatorColor()); updateStickyScrollingControls(); } @@ -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()); @@ -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. */ @@ -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; @@ -398,7 +396,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 0e4363abf0a..1dece650d71 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 @@ -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; @@ -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); } @@ -105,6 +107,9 @@ public void testCorrectColorsApplied() { StyledText stickyLineText = getStickyLineText(); assertEquals(backgroundColor, stickyLineText.getBackground()); + + Composite stickyLineSeparator = getStickyLineSeparator(); + assertEquals(separatorColor, stickyLineSeparator.getBackground()); } @Test @@ -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(); @@ -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(); @@ -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; } } @@ -286,4 +293,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 17bb7e5e60b..b10ee9ccfdd 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 @@ -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; } }