@mikhailru unfortunately, this patch won't solve this general problem we have in other situations where a default dark plot theme is used. To see this, go to the settings and select a darkish default theme for worksheets and create a new worksheet+plot. The axis label won't be clearly visible. We need to re-think how we treat QTextEdit::toHtml() in LabelWidget.cpp and how we plot this rich-text formatted string in TextLabel.cpp via QPainter::drawStaticText()...

@asemke , could you please describe the steps to reproduce wrong behavior with default dark worksheet theme? i tried setting a dark theme ("Dark") and axis label color seemed OK to me, screenshots attached (before and after patch).

In D16839#360121, @mikhailru wrote:

@asemke , could you please describe the steps to reproduce wrong behavior with default dark worksheet theme? i tried setting a dark theme ("Dark") and axis label color seemed OK to me, screenshots attached (before and after patch).

@mikhailru sorry, I was not very precise in my last comment. The problem that I referred to can be seen on these screenshots:

Observe the colors shown in the textedit in the properties dock widget. Do you also have this? This doesn't look nice and this is what I meant with "We need to re-think how we treat QTextEdit::toHtml() in LabelWidget.cpp and how we plot this rich-text formatted string in TextLabel.cpp via QPainter::drawStaticText()".

On the last screenshot I started with a "white desktop/white (default) worksheet" and applied the theme "BlackOnWhite" on the worksheet - only after this the foreground color of the label is set correctly.

You patch doesn't fix the problem for me, for white worksheet and plot the axis title is still hardly visible and I don't really understand why this should help. All properties of the text like forground and background colors are read from the html text in QTextEdit and set to QStaticText which is then painted in TextLabelPrivate::paint() via QPainter::drawStaticText(). A global color for the pen doesn't make much sense since the different parts of the label string can have different colors - this is what is encoded in the html code that is painted via the QStaticText.

@asemke, thanks for your explanation! I've done some research, and it looks like at least 3 issues exist:

(1) The first case is when theme == "". This is what happens when no theme has been configured via the settings dialog and labplot2rc does not contain "Theme" key in [Settings_Worksheet] group, or when labplot2rc does not exist (first run). In this case, CartesianPlot does not apply theme to children, including axes labels:

//in CartesianPlot::childAdded
if (!d->theme.isEmpty()) {
        const auto* elem = dynamic_cast<const WorksheetElement*>(child);
	if (elem) {
		KConfig config(ThemeHandler::themeFilePath(d->theme), KConfig::SimpleConfig);
		const_cast<WorksheetElement*>(elem)->loadThemeConfig(config);
	}
} else {
	//no theme is available, apply the default colors for curves only, s.a. XYCurve::loadThemeConfig()

This results in d->textWrapper.text in TextLabels containing plain text without any formatting. I've checked XMLs to verify:

<textLabel creation_time="2018-23-11 00:20:41:571" name="x axis 1">
<comment></comment>
<geometry x="-10.5833" y="605.444" horizontalPosition="3" verticalPosition="3" horizontalAlignment="1" verticalAlignment="1" rotationAngle="0" visible="1"/>
<text>x axis 1</text>
<format teXUsed="0" fontFamily="Computer Modern" fontSize="-1" fontPointSize="12" fontWeight="50" fontItalic="0" teXFontColor_r="0" teXFontColor_g="0" teXFontColor_b="0"/>
</textLabel>

Compare this to the XML obtained with theme != "":

<textLabel creation_time="2018-23-11 00:31:30:976" name="x axis 1">
<comment />
    <geometry x="-10.5833" y="605.444" horizontalPosition="3" verticalPosition="3" horizontalAlignment="1" verticalAlignment="1" rotationAngle="0" visible="1" />
    <text>
          &lt;!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN" "http://www.w3.org/TR/REC-html40/strict.dtd"&gt;
          &lt;html&gt;&lt;head&gt;&lt;meta name="qrichtext" content="1" /&gt;&lt;style type="text/css"&gt;
          p, li { white-space: pre-wrap; }
          &lt;/style&gt;&lt;/head&gt;&lt;body style=" font-family:'Noto Sans'; font-size:10pt; font-weight:400; font-style:normal;"&gt;
          &lt;p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px;"&gt;&lt;span style=" color:#ffffff; background-color:#000000;"&gt;x axis 1&lt;/span&gt;&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;
    </text>
    <format teXUsed="0" fontFamily="Computer Modern" fontSize="-1" fontPointSize="12" fontWeight="50" fontItalic="0" teXFontColor_r="0" teXFontColor_g="0" teXFontColor_b="0" />
</textLabel>

This text without formatting is painted with the default color, taken from the Qt theme. And this is the case which my original patch addressed.

(2) theme == "None". I suppose it's what you were talking about in "On the last screenshot...". In this case theme != "", but ThemeHandler::themeFilePath cannot find theme file and returns "", so values hardcoded in KConfigGroup::readEntry calls are used. Unfortunately, both [Label]/FontColor (TextLabel.cpp:842) and [Worksheet]/BackgroundFirstColor (Worksheet.cpp:96) use Qt::white as default.

(3) LabelWidget doesn't look nice with some Qt theme/label colors combination.

What can be done about this? I'm not deeply familiar with the codebase, but I have some ideas. If you find them worthwhile, I'll submit patches.

(1) I'm not sure whether using labels text without formatting is a supposed mode of operation. Maybe we should somehow fallback to "None" (hardcoded) theme instead of current behaviour?

(2) Swap background/foreground color defaults in TextLabel::loadThemeConfig. This one looks harmless.

(3) Concerning this one, I can't come up with a solution which doesn't hurt present functionality. I'll think a bit more. Btw, does [Label]/BackgroundColor affect plot appearance in any way?

In D16839#365584, @mikhailru wrote:

@asemke, thanks for your explanation! I've done some research, and it looks like at least 3 issues exist:

(1) The first case is when theme == "". This is what happens when no theme has been configured via the settings dialog and labplot2rc does not contain "Theme" key in [Settings_Worksheet] group, or when labplot2rc does not exist (first run). In this case, CartesianPlot does not apply theme to children, including axes labels:

//in CartesianPlot::childAdded
if (!d->theme.isEmpty()) {
        const auto* elem = dynamic_cast<const WorksheetElement*>(child);
	if (elem) {
		KConfig config(ThemeHandler::themeFilePath(d->theme), KConfig::SimpleConfig);
		const_cast<WorksheetElement*>(elem)->loadThemeConfig(config);
	}
} else {
	//no theme is available, apply the default colors for curves only, s.a. XYCurve::loadThemeConfig()

This results in d->textWrapper.text in TextLabels containing plain text without any formatting. I've checked XMLs to verify:

<textLabel creation_time="2018-23-11 00:20:41:571" name="x axis 1">
<comment></comment>
<geometry x="-10.5833" y="605.444" horizontalPosition="3" verticalPosition="3" horizontalAlignment="1" verticalAlignment="1" rotationAngle="0" visible="1"/>
<text>x axis 1</text>
<format teXUsed="0" fontFamily="Computer Modern" fontSize="-1" fontPointSize="12" fontWeight="50" fontItalic="0" teXFontColor_r="0" teXFontColor_g="0" teXFontColor_b="0"/>
</textLabel>

Compare this to the XML obtained with theme != "":

<textLabel creation_time="2018-23-11 00:31:30:976" name="x axis 1">
<comment />
    <geometry x="-10.5833" y="605.444" horizontalPosition="3" verticalPosition="3" horizontalAlignment="1" verticalAlignment="1" rotationAngle="0" visible="1" />
    <text>
          &lt;!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN" "http://www.w3.org/TR/REC-html40/strict.dtd"&gt;
          &lt;html&gt;&lt;head&gt;&lt;meta name="qrichtext" content="1" /&gt;&lt;style type="text/css"&gt;
          p, li { white-space: pre-wrap; }
          &lt;/style&gt;&lt;/head&gt;&lt;body style=" font-family:'Noto Sans'; font-size:10pt; font-weight:400; font-style:normal;"&gt;
          &lt;p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px;"&gt;&lt;span style=" color:#ffffff; background-color:#000000;"&gt;x axis 1&lt;/span&gt;&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;
    </text>
    <format teXUsed="0" fontFamily="Computer Modern" fontSize="-1" fontPointSize="12" fontWeight="50" fontItalic="0" teXFontColor_r="0" teXFontColor_g="0" teXFontColor_b="0" />
</textLabel>

This text without formatting is painted with the default color, taken from the Qt theme. And this is the case which my original patch addressed.

Ok. This makes sense. But let's use maybe the color QApplication::palette().color(QPalette::Active,QPalette::Text ) ) instead of hard-coded Qt::black.

(2) theme == "None". I suppose it's what you were talking about in "On the last screenshot...". In this case theme != "", but ThemeHandler::themeFilePath cannot find theme file and returns "", so values hardcoded in KConfigGroup::readEntry calls are used. Unfortunately, both [Label]/FontColor (TextLabel.cpp:842) and [Worksheet]/BackgroundFirstColor (Worksheet.cpp:96) use Qt::white as default.

Thanks for digging into this. I just debugged a bit, too. Having "None" here is a bug. We should save an empty string if "None" was selected. I'll fix this soon.

(3) LabelWidget doesn't look nice with some Qt theme/label colors combination.

What can be done about this? I'm not deeply familiar with the codebase, but I have some ideas. If you find them worthwhile, I'll submit patches.

(1) I'm not sure whether using labels text without formatting is a supposed mode of operation. Maybe we should somehow fallback to "None" (hardcoded) theme instead of current behaviour?

Having text without formatting is perfectly fine. User simply types in some text without the need for additional formatting. Once this bug with "None" is fixed and we have empty string for the theme name, the theme settings with hard-coded default values won't be loaded anymore as you described above. Together with the default Qt theme color for the painter as you originally proposed we should be ok here.

(2) Swap background/foreground color defaults in TextLabel::loadThemeConfig. This one looks harmless.

This shouldn't be needed anymore once we adressed the two points mentioned above.

(3) Concerning this one, I can't come up with a solution which doesn't hurt present functionality. I'll think a bit more. Btw, does [Label]/BackgroundColor affect plot appearance in any way?

In principle it should be possible to change the background color for different parts of the text as shown on this screenshot:

In D16839#370476, @asemke wrote:

In D16839#365584, @mikhailru wrote:

This text without formatting is painted with the default color, taken from the Qt theme. And this is the case which my original patch addressed.

Ok. This makes sense. But let's use maybe the color QApplication::palette().color(QPalette::Active,QPalette::Text ) ) instead of hard-coded Qt::black.

Having thought more about this, for the unformatted text a better idea would to use Qt::black for the foreground color in case no theme is active and to use the foreground color from the theme, otherwise. Furthermore, for unformatted texts we shouldn't add any formatting when we apply a theme. At the moment we convert plain text to a html-formatted string which causes problems and is not required for plain text. It should be enough if we only set the proper pen color.

In D16839#370541, @asemke wrote:

In D16839#370476, @asemke wrote:

In D16839#365584, @mikhailru wrote:

This text without formatting is painted with the default color, taken from the Qt theme. And this is the case which my original patch addressed.

Ok. This makes sense. But let's use maybe the color QApplication::palette().color(QPalette::Active,QPalette::Text ) ) instead of hard-coded Qt::black.

Having thought more about this, for the unformatted text a better idea would to use Qt::black for the foreground color in case no theme is active and to use the foreground color from the theme, otherwise. Furthermore, for unformatted texts we shouldn't add any formatting when we apply a theme. At the moment we convert plain text to a html-formatted string which causes problems and is not required for plain text. It should be enough if we only set the proper pen color.

I just pushed a change e3bafa4bac89a49a1ef58bfd78d17429750f1c45 which should address many of these issues. @mikhailru can you please check the current master? Applying different colors to different parts of the text and showing the correct color in the properties dock widget is still a problem in some cases. I'll check this.

I just pushed a change e3bafa4bac89a49a1ef58bfd78d17429750f1c45 which should address many of these issues. @mikhailru can you please check the current master? Applying different colors to different parts of the text and showing the correct color in the properties dock widget is still a problem in some cases. I'll check this.

@asemke, I've just checked master and can confirm that all the issues I mentioned are resolved. I believe the bug should be closed now (if I should close it myself, please tell; I'm not quite used to the kde workflow yet).

PS
Extra thanks for the //if no selection is done, apply the new color for the whole label part! I find this much more intuitive and convenient.

In D16839#372556, @mikhailru wrote:

@asemke, I've just checked master and can confirm that all the issues I mentioned are resolved. I believe the bug should be closed now (if I should close it myself, please tell; I'm not quite used to the kde workflow yet).

PS
Extra thanks for the //if no selection is done, apply the new color for the whole label part! I find this much more intuitive and convenient.

I closed the bug ticket now. Thank you for raising this issue and helping to sort out the thoughts and different options here. We can close this revision I think.