Skip to content

Commit

Permalink
Use Jenkins theme CSS color variables for unset states
Browse files Browse the repository at this point in the history 
The original support for negative/inverse text uses a combination off
`currentColor` and a bright-level-7 value. When using a default Jenkins
theme, these color definitions work as expected. However, clients using
the dark theme may not be able to read the text since both the text and
background are a bright color.

Modern Jenkins themes provide CSS color variables. Instead of trying to
rely on `currentColor`, use the default text color (`var(--text-color)`)
and the default background color (`var(--background)`). This should
allow negative/inverse text to render appropriately no matter what
theme mode is used.

Signed-off-by: James Knight <[email protected]>
  • Loading branch information
@jdknight
jdknight committed Nov 11, 2024
1 parent bc8759a commit 27b16e5
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 80 deletions.
54 changes: 6 additions & 48 deletions src/main/java/hudson/plugins/ansicolor/AnsiHtmlOutputStream.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,27 +313,8 @@ private String getDefaultForegroundColor() {
Integer defaultFgIndex = colorMap.getDefaultForeground();
if (defaultFgIndex != null) color = colorMap.getNormal(defaultFgIndex);
if (color == null) {
// with no default foreground set, we need to guess about (currently happened in xterm and css themes)
// possible approaches are:
// • hardcoded black "#000000"
// • colorMap.getNormal(0)
// • "currentColor" → see also: http://stackoverflow.com/a/42586457/2880699
// but note, that this approach only works, if all <span style="color: …"> are currently closed

// Note that in my default szenario with, the default text color is #333333. Tested with:
// • Jenkins 1.6.5.3, AnsiColor plugin 0.4.3, xterm scheme
// • Firefox 51.0.1, Kubuntu 16.04
// • Chromium 56.0.2924.76, Kubuntu 16.04
// • Firefox 51.0.1, Windows XP

// So I finally decide for the "currentColor" approach, because:
// • It gives the best results for negative / inverse text (what is the major reason for implementing this function).
// • It looks also the best alternative, if e.g. someone customizes Jenkins colors.
// • Finally, the clause "only works, if all <span style="color: …"> are currently closed" is fulfilled for the negative / inverse case

// color = "#000000"; // hardcoded black
// color = colorMap.getNormal(0); // hardcoded index 0
color = "currentColor"; // see http://stackoverflow.com/a/42586457/2880699
// with no default foreground set, use the default theme text color
color = "var(--text-color)";
}
return color;
}
Expand All @@ -343,16 +324,8 @@ private String getDefaultBackgroundColor() {
Integer defaultBgIndex = colorMap.getDefaultBackground();
if (defaultBgIndex != null) color = colorMap.getNormal(defaultBgIndex);
if (color == null) {
// with no default foreground set, we need to guess about (currently happened in xterm and css themes)
// possible approaches are:
// • hardcoded white "#FFFFFF"
// • colorMap.getNormal(7)
// • colorMap.getBright(7)
// I finally decide for colorMap.getBright(7).

//color "#FFFFFF"; // hardcoded white
//color = colorMap.getNormal(7); // hardcoded normal index 7
color = colorMap.getBright(7); // hardcoded bright index 7
// with no default foreground set, use the default theme background color
color = "var(--background)";
}
return color;
}
Expand All @@ -362,22 +335,9 @@ private void setForegroundColor(String color) {
AnsiAttrType attrType = !swapColors ? AnsiAttrType.FG : AnsiAttrType.BG;
String attrName = !swapColors ? "color" : "background-color";
if (color == null && swapColors) color = getDefaultForegroundColor();
boolean restorebg = false;
if (swapColors && color.equals("currentColor")) {
// need also to temporarily unwind textcolor, to having correct access to the "currentColor" value
closeTagOfType(AnsiAttrType.FGBG);
restorebg = true;
} else {
closeTagOfType(attrType);
}
closeTagOfType(attrType);
if (color != null)
openTag(new AnsiAttributeElement(attrType, "span", "style=\"" + attrName + ": " + color + ";\""));
if (restorebg) {
// Because of the "currentColor" trick, we always need to use two seperate <span> tags for this case.
String bg = currentBackgroundColor;
if (bg == null) bg = getDefaultBackgroundColor();
openTag(new AnsiAttributeElement(AnsiAttrType.FG, "span", "style=\"color: " + bg + ";\""));
}
currentForegroundColor = color;
}

Expand Down Expand Up @@ -475,9 +435,7 @@ else switch (attribute) {
bg = tmp;
}
closeTagOfType(AnsiAttrType.FGBG);
if (fg != null && bg != null && !bg.equals("currentColor")) {
// In case of "currentColor" trick, we need to use two seperate <span> tags.
// But if not, then we can use one single <span> tag to set both background and foreground color.
if (fg != null && bg != null) {
openTag(new AnsiAttributeElement(AnsiAttrType.FGBG, "span", "style=\"background-color: " + bg + "; color: " + fg + ";\""));
} else {
if (bg != null) openTag(new AnsiAttributeElement(AnsiAttrType.BG, "span", "style=\"background-color: " + bg + ";\""));
Expand Down
58 changes: 26 additions & 32 deletions src/test/java/hudson/plugins/ansicolor/AnsiHtmlOutputStreamTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,34 +115,33 @@ public void testNegative() throws IOException {
// simple tests
assertThatAnnotateIs(
"\033[7mon\033[moff",
"<span style=\"background-color: currentColor;\"><span style=\"color: #FFFFFF;\">on</span></span>off");
"<span style=\"background-color: var(--text-color); color: var(--background);\">on</span>off");

assertThatAnnotateIs(
"\033[7mon\033[27moff",
"<span style=\"background-color: currentColor;\"><span style=\"color: #FFFFFF;\">on</span></span>off");
"<span style=\"background-color: var(--text-color); color: var(--background);\">on</span>off");

assertThatAnnotateIs(
"\033[33;7mon\033[27moff",
"<span style=\"color: #CDCD00;\"></span>" + // unnecessary <span> tag, could be removed …
"<span style=\"background-color: #CDCD00; color: #FFFFFF;\">on</span>" +
"<span style=\"background-color: #CDCD00; color: var(--background);\">on</span>" +
"<span style=\"color: #CDCD00;\">off</span>");

assertThatAnnotateIs(
"\033[7;33mon\033[27moff",
"<span style=\"background-color: currentColor;\"><span style=\"color: #FFFFFF;\"></span></span>" + // unnecessary <span> tag, could be removed …
"<span style=\"color: #FFFFFF;\"><span style=\"background-color: #CDCD00;\">on</span></span>" + // could be optimized to be a single <span> tag
"<span style=\"background-color: var(--text-color); color: var(--background);\">" +
"<span style=\"background-color: #CDCD00;\">on</span></span>" +
"<span style=\"color: #CDCD00;\">off</span>");

assertThatAnnotateIs(
"\033[41;7mon\033[27moff",
"<span style=\"background-color: #CD0000;\"></span>" + // unnecessary <span> tag, could be removed …
"<span style=\"background-color: currentColor;\"><span style=\"color: #CD0000;\">on</span></span>" +
"<span style=\"background-color: var(--text-color); color: #CD0000;\">on</span>" +
"<span style=\"background-color: #CD0000;\">off</span>");

assertThatAnnotateIs(
"\033[7;41mon\033[27moff",
"<span style=\"background-color: currentColor;\">" +
"<span style=\"color: #FFFFFF;\"></span>" + // unnecessary <span> tag, could be removed …
"<span style=\"background-color: var(--text-color); color: var(--background);\">" +
"<span style=\"color: #CD0000;\">on</span>" +
"</span>" +
"<span style=\"background-color: #CD0000;\">off</span>");
Expand All @@ -155,70 +154,65 @@ public void testNegative() throws IOException {

assertThatAnnotateIs(
"\033[7;33;41mon\033[27moff",
"<span style=\"background-color: currentColor;\"><span style=\"color: #FFFFFF;\"></span></span>" + // unnecessary <span> tag, could be removed …
"<span style=\"color: #FFFFFF;\"><span style=\"background-color: #CDCD00;\"></span></span>" + // unnecessary <span> tag, could be removed …
"<span style=\"background-color: #CDCD00;\"><span style=\"color: #CD0000;\">on</span></span>" + // could be optimized to be a single <span> tag
"<span style=\"background-color: var(--text-color); color: var(--background);\">" +
"<span style=\"background-color: #CDCD00;\"><span style=\"color: #CD0000;\">on</span></span></span>" + // could be optimized to be a single <span> tag
"<span style=\"background-color: #CD0000; color: #CDCD00;\">off</span>");


// reset foreground / background to default while [7m is active
assertThatAnnotateIs(
"\033[33;7mon\033[39mdefault",
"<span style=\"color: #CDCD00;\"></span>" + // unnecessary <span> tag, could be removed …
"<span style=\"background-color: #CDCD00; color: #FFFFFF;\">on</span>" +
"<span style=\"background-color: currentColor;\"><span style=\"color: #FFFFFF;\">default</span></span>");
"<span style=\"background-color: #CDCD00; color: var(--background);\">on" +
"<span style=\"background-color: var(--text-color);\">default</span></span>");

assertThatAnnotateIs(
"\033[7;33mon\033[39mdefault",
"<span style=\"background-color: currentColor;\"><span style=\"color: #FFFFFF;\"></span></span>" + // unnecessary <span> tag, could be removed …
"<span style=\"color: #FFFFFF;\"><span style=\"background-color: #CDCD00;\">on</span></span>" + // could be optimized to be a single <span> tag
"<span style=\"background-color: currentColor;\"><span style=\"color: #FFFFFF;\">default</span></span>");
"<span style=\"background-color: var(--text-color); color: var(--background);\">" +
"<span style=\"background-color: #CDCD00;\">on</span>" +
"<span style=\"background-color: var(--text-color);\">default</span></span>");

assertThatAnnotateIs(
"\033[41;7mon\033[49mdefault",
"<span style=\"background-color: #CD0000;\"></span>" + // unnecessary <span> tag, could be removed …
"<span style=\"background-color: currentColor;\">" +
"<span style=\"color: #CD0000;\">on</span>" +
"<span style=\"color: #FFFFFF;\">default</span>" +
"<span style=\"background-color: var(--text-color); color: #CD0000;\">on" +
"<span style=\"color: var(--background);\">default</span>" +
"</span>");

assertThatAnnotateIs(
"\033[7;41mon\033[49mdefault",
"<span style=\"background-color: currentColor;\">" +
"<span style=\"color: #FFFFFF;\"></span>" + // unnecessary <span> tag, could be removed …
"<span style=\"background-color: var(--text-color); color: var(--background);\">" +
"<span style=\"color: #CD0000;\">on</span>" +
"<span style=\"color: #FFFFFF;\">default</span>" +
"<span style=\"color: var(--background);\">default</span>" +
"</span>");

assertThatAnnotateIs(
"\033[33;41;7mon\033[39mdefault",
"<span style=\"color: #CDCD00;\"><span style=\"background-color: #CD0000;\"></span></span>" + // unnecessary <span> tag, could be removed …
"<span style=\"background-color: #CDCD00; color: #CD0000;\">on</span>" +
"<span style=\"background-color: currentColor;\"><span style=\"color: #CD0000;\">default</span></span>");
"<span style=\"background-color: #CDCD00; color: #CD0000;\">on" +
"<span style=\"background-color: var(--text-color);\">default</span></span>");

assertThatAnnotateIs(
"\033[7;33;41mon\033[39mdefault",
"<span style=\"background-color: currentColor;\"><span style=\"color: #FFFFFF;\"></span></span>" + // unnecessary <span> tag, could be removed …
"<span style=\"color: #FFFFFF;\"><span style=\"background-color: #CDCD00;\"></span></span>" + // unnecessary <span> tag, could be removed …
"<span style=\"background-color: var(--text-color); color: var(--background);\">" +
"<span style=\"background-color: #CDCD00;\"><span style=\"color: #CD0000;\">on</span></span>" +
"<span style=\"background-color: currentColor;\"><span style=\"color: #CD0000;\">default</span></span>");
"<span style=\"color: #CD0000;\"><span style=\"background-color: var(--text-color);\">default</span></span></span>");

assertThatAnnotateIs(
"\033[33;41;7mon\033[49mdefault",
"<span style=\"color: #CDCD00;\"><span style=\"background-color: #CD0000;\"></span></span>" + // unnecessary <span> tag, could be removed …
"<span style=\"background-color: #CDCD00; color: #CD0000;\">" +
"on" +
"<span style=\"color: #FFFFFF;\">default</span>" +
"<span style=\"color: var(--background);\">default</span>" +
"</span>");

assertThatAnnotateIs(
"\033[7;33;41mon\033[49mdefault",
"<span style=\"background-color: currentColor;\"><span style=\"color: #FFFFFF;\"></span></span>" + // unnecessary <span> tag, could be removed …
"<span style=\"color: #FFFFFF;\"><span style=\"background-color: #CDCD00;\"></span></span>" + // unnecessary <span> tag, could be removed …
"<span style=\"background-color: var(--text-color); color: var(--background);\">" +
"<span style=\"background-color: #CDCD00;\">" +
"<span style=\"color: #CD0000;\">on</span>" +
"<span style=\"color: #FFFFFF;\">default</span>" +
"</span>");
"<span style=\"color: var(--background);\">default</span>" +
"</span></span>");


// simple tests with dark theme, as there has been default foreground / background colors defined (in contrast to xterm scheme)
Expand Down

0 comments on commit 27b16e5

Please sign in to comment.