Skip to content

brand - warn on invalid color name usage #12537

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 14, 2025
Merged

brand - warn on invalid color name usage #12537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2d21d0f
brand - warn on invalid color name usage
cscheid Apr 14, 2025
280e7ee
test - add a simple check for warning only for now
cderv Apr 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/core/brand/brand.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import {
import { InternalError } from "../lib/error.ts";

import { join, relative } from "../../deno_ral/path.ts";
import { warnOnce } from "../log.ts";
import { isCssColorName } from "../css/color-names.ts";

// we can't programmatically convert typescript types to string arrays,
// so we have to define this manually. They should match `BrandNamedThemeColor` in schema-types.ts
Expand Down Expand Up @@ -176,7 +178,7 @@ export class Brand {
// - if the name is in the "palette" key, use that value as they key for a recursive call (so color names can be aliased or redefined away from scss defaults)
// - if the name is a default color name, call getColor recursively (so defaults can use named values)
// - otherwise, assume it's a color value and return it
getColor(name: string): string {
getColor(name: string, quiet = false): string {
const seenValues = new Set<string>();

do {
Expand All @@ -198,6 +200,12 @@ export class Brand {
) {
name = this.data.color[name as BrandNamedThemeColor]!;
} else {
// if the name is not a default color name, assume it's a color value
if (!isCssColorName(name) && !quiet) {
warnOnce(
`"${name}" is not a valid CSS color name.\nThis might cause SCSS compilation to fail, or the color to have no effect.`,
);
}
return name;
}
} while (seenValues.size < 100); // 100 ought to be enough for anyone, with apologies to Bill Gates
Expand Down
163 changes: 163 additions & 0 deletions src/core/css/color-names.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
/*
* color-names.ts
*
* Copyright (C) 2025 Posit Software, PBC
*/

// https://www.w3.org/TR/css-color-3/#svg-color
export const isCssColorName = (name: string): boolean => {
if (name.startsWith("#")) return true;
if (name.startsWith("rgb")) return true;
if (name.startsWith("hsl")) return true;
if (name.startsWith("hwb")) return true;
Copy link
Contributor

@gordonwoodhull gordonwoodhull Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a lot more color spaces than this, but this should be fine.


return [
"aliceblue",
"antiquewhite",
"aqua",
"aquamarine",
"azure",
"beige",
"bisque",
"black",
"blanchedalmond",
"blue",
"blueviolet",
"brown",
"burlywood",
"cadetblue",
"chartreuse",
"chocolate",
"coral",
"cornflowerblue",
"cornsilk",
"crimson",
"cyan",
"darkblue",
"darkcyan",
"darkgoldenrod",
"darkgray",
"darkgreen",
"darkgrey",
"darkkhaki",
"darkmagenta",
"darkolivegreen",
"darkorange",
"darkorchid",
"darkred",
"darksalmon",
"darkseagreen",
"darkslateblue",
"darkslategray",
"darkslategrey",
"darkturquoise",
"darkviolet",
"deeppink",
"deepskyblue",
"dimgray",
"dimgrey",
"dodgerblue",
"firebrick",
"floralwhite",
"forestgreen",
"fuchsia",
"gainsboro",
"ghostwhite",
"gold",
"goldenrod",
"gray",
"green",
"greenyellow",
"grey",
"honeydew",
"hotpink",
"indianred",
"indigo",
"ivory",
"khaki",
"lavender",
"lavenderblush",
"lawngreen",
"lemonchiffon",
"lightblue",
"lightcoral",
"lightcyan",
"lightgoldenrodyellow",
"lightgray",
"lightgreen",
"lightgrey",
"lightpink",
"lightsalmon",
"lightseagreen",
"lightskyblue",
"lightslategray",
"lightslategrey",
"lightsteelblue",
"lightyellow",
"lime",
"limegreen",
"linen",
"magenta",
"maroon",
"mediumaquamarine",
"mediumblue",
"mediumorchid",
"mediumpurple",
"mediumseagreen",
"mediumslateblue",
"mediumspringgreen",
"mediumturquoise",
"mediumvioletred",
"midnightblue",
"mintcream",
"mistyrose",
"moccasin",
"navajowhite",
"navy",
"oldlace",
"olive",
"olivedrab",
"orange",
"orangered",
"orchid",
"palegoldenrod",
"palegreen",
"paleturquoise",
"palevioletred",
"papayawhip",
"peachpuff",
"peru",
"pink",
"plum",
"powderblue",
"purple",
"red",
"rosybrown",
"royalblue",
"saddlebrown",
"salmon",
"sandybrown",
"seagreen",
"seashell",
"sienna",
"silver",
"skyblue",
"slateblue",
"slategray",
"slategrey",
"snow",
"springgreen",
"steelblue",
"tan",
"teal",
"thistle",
"tomato",
"turquoise",
"violet",
"wheat",
"white",
"whitesmoke",
"yellow",
"yellowgreen",
Copy link
Contributor

@gordonwoodhull gordonwoodhull Apr 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, looks the same as the named colors in typst_css.lua.

transparent is also important, once a keyword, now a color

I see both of us missed rebeccapurple, the other color defined after the original spec.

].includes(name);
};
23 changes: 11 additions & 12 deletions src/core/sass/brand.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ import {
BrandFontWeight,
} from "../../resources/types/schema-types.ts";
import { Brand } from "../brand/brand.ts";
import {
darkModeDefault,
} from "../../format/html/format-html-info.ts";

import { darkModeDefault } from "../../format/html/format-html-info.ts";

const defaultColorNameMap: Record<string, string> = {
"link-color": "link",
Expand Down Expand Up @@ -81,8 +78,8 @@ export async function brandBootstrapSassBundles(
dependency: "bootstrap",
user: layers.light,
dark: {
user: layers.dark
}
user: layers.dark,
},
}];
}

Expand Down Expand Up @@ -187,7 +184,7 @@ const brandColorLayer = (

// format-specific name mapping
for (const [key, value] of Object.entries(nameMap)) {
const resolvedValue = brand.getColor(value);
const resolvedValue = brand.getColor(value, true);
if (resolvedValue !== value) {
colorVariables.push(
`$${key}: ${resolvedValue} !default;`,
Expand Down Expand Up @@ -579,7 +576,7 @@ export async function brandSassLayers(
const brand = await project.resolveBrand(fileName);
const sassLayers: LightDarkSassLayers = {
light: [],
dark: []
dark: [],
};

for (const mode of ["light", "dark"] as Array<"dark" | "light">) {
Expand Down Expand Up @@ -660,10 +657,12 @@ export async function brandSassFormatExtras(
key: "brand",
dependency: "bootstrap",
user: htmlSassBundleLayers.light,
dark: htmlSassBundleLayers.dark.length ? {
user: htmlSassBundleLayers.dark,
default: darkModeDefault(format.metadata)
} : undefined
dark: htmlSassBundleLayers.dark.length
? {
user: htmlSassBundleLayers.dark,
default: darkModeDefault(format.metadata),
}
: undefined,
},
],
},
Expand Down
27 changes: 27 additions & 0 deletions tests/docs/smoke-all/2025/04/14/11665.qmd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
title: Hello, `_brand.yml`!
brand:
typography:
monospace:
color: foreground
format: html
_quarto:
tests:
html:
noErrors: default
printsMessage:
level: WARN
regex: 'foreground.*valid CSS color name'
---

## Links

Download the Positron Beta here: <https://positron.posit.co/download.html>

Docs for `_brand.yml`: <https://posit-dev.github.io/brand-yml/>, <https://quarto.org/docs/authoring/brand.html>

## Placeholder text

This is an example document with a [number](https://quarto.org) of `features` to help you come with a nicely branded document.

Duis ornare ex ac iaculis pretium. Maecenas sagittis odio id erat pharetra, sit amet consectetur quam sollicitudin. Vivamus pharetra quam purus, nec sagittis risus pretium at. Nullam feugiat, turpis ac accumsan interdum, sem tellus blandit neque, id vulputate diam quam semper nisl. Donec sit amet enim at neque porttitor aliquet. Phasellus facilisis nulla eget placerat eleifend. Vestibulum non egestas eros, eget lobortis ipsum. Nulla rutrum massa eget enim aliquam, id porttitor erat luctus. Nunc sagittis quis eros eu sagittis. Pellentesque dictum, erat at pellentesque sollicitudin, justo augue pulvinar metus, quis rutrum est mi nec felis. Vestibulum efficitur mi lorem, at elementum purus tincidunt a. Aliquam finibus enim magna, vitae pellentesque erat faucibus at. Nulla mauris tellus, imperdiet id lobortis et, dignissim condimentum ipsum. Morbi nulla orci, varius at aliquet sed, facilisis id tortor. Donec ut urna nisi.
Loading