Commit 8d2582b0 authored by snooxx's avatar snooxx Committed by Tomaz Canabrava
Browse files

Adapt scrollbar to terminal color scheme for Breeze widget style again

2a71f063 changed the scrollbar background to match the terminal color
scheme. This looks fine for the Breeze widget style, but fails for
non-Breeze styles such as Fusion or Plastique. Sometimes scrollbars
would be only barely visible, in other cases the scrollbar would look
really out of place (e.g. very dark scrollbar on white terminal
background with light widget colors).

92e19c63 then unconditionally removed the color matching functionality,
so scrollbars would always use the (as of dc8ad830 application-specific)
widget color scheme. This solves the accessibily issues, but might be
considered a step backwards for Breeze trying to blend into the app's content.

In !402 adding a configuration toggle to let users disable color
matching was turned down. Instead, automatically switching behavior
depending on the current widget style was suggested. Obviously this is
less than ideal from a maintenance perspective and might look like magic
for users, but seems like the best option left at this point.

After this patch, Breeze users will get scrollbar colors matched to the
terminal color scheme as intended in 2a71f063. Scrollbars of non-Breeze
users will be colored according to the widget color scheme as
implemented in 92e19c63, which is also how it looked before 2a71f063
landed. Changing widget styles at runtime is fully supported.

Implementation-wise, note that `MenuStyle` is a `QProxyStyle` applied to
the whole application, which slightly affects how to determine the
current widget style in use.

In the future, styles shall only be added to the allowlist if they look
good with both light and dark terminal background colors (e.g Fusion
currently does not qualify, because while the scrollbar might look
acceptable with a dark background, the coloring algorithm in Konsole
does not provide adequate scrollbar coloring with light backgrounds).

Test Plan:
- start Konsole with Breeze widget style
- change background color in terminal profile from black to white:
  scrollbar color should match terminal background
- change application color scheme from light to dark: scrollbar color
  should be unaffected
- change widget style to Fusion in "kcmshell5 kcm_style"
- change background color in terminal profile from white to black:
  scrollbar color should be unaffected
- change application color scheme from dark to light: scrollbar color
  should match widget colors
(The last step will only work without restarting Konsole once the third
patch in this series has been applied.)
parent 3e652002
......@@ -134,13 +134,7 @@ void TerminalColor::onColorsChanged()
palette.setColor(QPalette::WindowText, buttonTextColor);
palette.setColor(QPalette::ButtonText, buttonTextColor);
QWidget *widget = qobject_cast<QWidget *>(parent());
widget->setPalette(palette);
Q_EMIT onPalette(palette);
widget->update();
}
void TerminalColor::swapFGBGColors()
......
......@@ -320,7 +320,7 @@ TerminalDisplay::TerminalDisplay(QWidget *parent)
connect(KonsoleSettings::self(), &KonsoleSettings::configChanged, this, &TerminalDisplay::setupHeaderVisibility);
_terminalColor = new TerminalColor(this);
connect(_terminalColor, &TerminalColor::onPalette, _scrollBar, &TerminalScrollBar::setPalette);
connect(_terminalColor, &TerminalColor::onPalette, _scrollBar, &TerminalScrollBar::updatePalette);
_terminalPainter = new TerminalPainter(this);
connect(this, &TerminalDisplay::drawContents, _terminalPainter, &TerminalPainter::drawContents);
......
......@@ -19,7 +19,9 @@
#include <KMessageWidget>
// Qt
#include <QGuiApplication>
#include <QLabel>
#include <QProxyStyle>
#include <QRect>
#include <QTimer>
......@@ -214,4 +216,31 @@ void TerminalScrollBar::scrollImage(int lines, const QRect &screenWindowRegion,
// scroll the display vertically to match internal _image
display->scroll(0, display->terminalFont()->fontHeight() * (-lines), scrollRect);
}
void TerminalScrollBar::changeEvent(QEvent *e)
{
if (e->type() == QEvent::StyleChange) {
updatePalette(_backgroundMatchingPalette);
}
QScrollBar::changeEvent(e);
}
void TerminalScrollBar::updatePalette(const QPalette &pal)
{
_backgroundMatchingPalette = pal;
auto proxyStyle = qobject_cast<const QProxyStyle *>(style());
const QStyle *appStyle = proxyStyle ? proxyStyle->baseStyle() : style();
// Scrollbars in widget styles like Fusion or Plastique do not work well with custom
// scrollbar coloring, in particular in conjunction with light terminal background colors.
// Use custom colors only for widget styles matched by the allowlist below, otherwise
// fall back to generic widget colors.
if (appStyle->objectName() == QLatin1String("breeze")) {
setPalette(_backgroundMatchingPalette);
} else {
setPalette(QGuiApplication::palette());
}
}
} // namespace Konsole
......@@ -84,6 +84,10 @@ public:
return _highlightScrolledLines;
}
void changeEvent(QEvent *e) override;
void updatePalette(const QPalette &pal);
public Q_SLOTS:
void scrollBarPositionChanged(int value);
......@@ -94,6 +98,7 @@ private:
bool _alternateScrolling;
Enum::ScrollBarPositionEnum _scrollbarLocation;
HighlightScrolledLines _highlightScrolledLines;
QPalette _backgroundMatchingPalette;
};
} // namespace Konsole
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment