Reduce number of generated CSS classes for each Type, Status, Meting State, etc... by klaustopher · Pull Request #24878 · opf/openproject

klaustopher · 2026-08-21T16:07:42Z

Ticket

https://community.openproject.org/projects/OP/work_packages/OP-16772/activity

Revives the spike from #18297 (closed), rebuilt on current dev and carried through to every call site.

What are you trying to accomplish?

app/views/highlighting/styles.css.erb generated a full matrix of highlighting classes: for every status, priority, type, phase definition, meeting/project status and color, it emitted seven rules, each repeating a ~470 byte block of six RGB/HSL custom properties plus the theme variables.

On a modest data set like the seeded DEV environment (148 colors, 14 statuses, 9 types, 4 priorities, 4 phase definitions) that is 261,808 bytes across 1185 rules, and it grows linearly with every status, type or color an instance defines. Every page load pulls it in.

After this PR the generated stylesheet contains nothing but data:

.__hl_status_1 { --hl-color: #1098AD; --hl-perceived-lightness: 0.4886 }

261,808 → 14,066 bytes, 1185 → 190 rules (18.6× smaller).

It also fixes a caching bug that fell out of the same design — see below.

What approach did you choose and why?

Separate the data from the styling

The old design had the split in an awkward place. The generated stylesheet decided what a highlight looks like per resource, while _colors.sass decided which resources get a dot using attribute prefix matching with exclusion lists:

[class^='__hl_inline_']:not([class^='__hl_inline_type'], [class^='__hl_inline_project_phase_definition'])

Now the generated stylesheet emits one rule per resource declaring only the color, and a fixed set of ~10 static rules in _colors.sass consumes it: __hl_background, __hl_foreground, __hl_dot, __hl_border_top. Both classes land on the same element, so the color is in scope.

The exclusion lists are gone. Nothing gets a dot unless it asks for one.

Two custom properties instead of six

The six properties existed to serve two jobs: --color-r/g/b for the flat fills and the luminance dot product, --color-h/s/l for lightness arithmetic that can't be done in sRGB.

Relative color syntax does both from a single hexcode:

background: var(--hl-color);                                   /* was rgb(r,g,b)              */
background: rgb(from var(--hl-color) r g b / 0.18);            /* was rgba(r,g,b,alpha)       */
border: 1px solid hsl(from var(--hl-color) h s calc(l - 15));  /* was hsl(h,s,calc((l-15)*1%)) */

Unitless calc(l - 15) is exactly equivalent to the old calc((var(--color-l) - 15) * 1%) — s and l resolve as 0–100 numbers. This is the form already used in _variable_defaults.scss:138, and relative color syntax already ships in three places in the codebase.

Perceived lightness is a pure function of the color, so computing it in CSS on every rule was wasted work. It now comes precomputed from a new Colors::HexColor#perceived_lightness, which sits next to brightness_yiq — it answers the same kind of question. That also drops ColorConversion from the helper entirely (the gem stays as a transitive dep of md_to_pdf).

--lightness-threshold and the switch/lighten-by calcs stay in CSS on purpose. Both thresholds are constants, so Ruby could emit finished values, but that drags theme decisions back into Ruby and busts the whole cache whenever a designer tweaks one. The seam is: Ruby emits facts about the color, CSS decides what to do with them.

Split the overloaded `__hl_inline_*`

__hl_inline_<name>_<id> meant one of two different things depending on how the resource happened to be generated — colored text for types and phase definitions, a leading dot for statuses and priorities. Call sites could not tell which they were asking for.

before	after
`__hl_inline_type_5`	`__hl_uppercase __hl_foreground __hl_type_5`
`__hl_inline_priority_5`	`__hl_dot __hl_priority_5`
`__hl_background_status_5`	`__hl_background __hl_status_5`
`__hl_inline_color_5_dot` / `_text`	`__hl_dot __hl_color_5` / `__hl_foreground __hl_color_5`

OpPrimer::StatusButtonComponent shows why this was worth removing: its single :inline style served both a leading visual icon and an option label. Those are now :foreground and :dot, which is what each was already rendering as, so the ambiguity resolves by call site rather than by lookup table.

Three frontend call sites take the property name at runtime and can't pick a treatment statically (op-autocompleter, global-search-input, highlighted-resource-display-field). Highlighting.inlineClass stays for them, now as an explicit dispatcher over one table — previously that table was implicit and spread across two files.

This also kills the '__hl_inline_type_ ' prefix-with-trailing-space hack in colors-autocompleter.component.ts, which existed only to trigger the attribute selectors.

Theme resolution moves into CSS, which fixes a caching bug

<body> already carries data-color-mode and data-light-theme/data-dark-theme (theme-utils.ts:50), so all four theme states are addressable in pure CSS. Light is the default and dark an override, because the theme controller only stamps those attributes once Stimulus connects.

That matters beyond tidiness. The old stylesheet branched on User.current.pref.dark_color_mode? while HighlightingController cached it under a key with no theme in it and served it public: true for a year. UserPreference sat in most_recently_changed to paper over that: since the version tag is part of the URL, any user changing any preference produced a new URL for everyone, which is a very blunt way to get one user a refetch after a theme switch.

The stylesheet no longer varies by user, so UserPreference is dropped and the existing expires_in 1.year, public: true becomes correct rather than a hazard. Color takes its place — editing a hexcode changed the output but invalidated nothing, since only the models referencing colors were listed.

…which surfaced a live bug in the controller

Removing UserPreference from the freshness key makes the version tag far more stable, so the Rails cache now actually gets hits. That turned out to matter, because the cache had never worked:

OpenProject::Cache.fetch("highlighting/styles", tag) do
  render template: "highlighting/styles", formats: [:css]
end

OpenProject::Cache.fetch discards the value its block returns, and the block is where the rendering happened. On a miss that works by side effect — render populates the response. On a hit the block never runs, so nothing is rendered, the action falls through to implicit rendering, and that cannot resolve a text/css template without the explicit formats:. The request dies with ActionController::UnknownFormat.

So the cache has never served a byte. Every hit is a hard failure, for any user, regardless of theme — I confirmed it with a probe against a real cache store, including two consecutive requests from the same user.

It stayed hidden behind the HTTP layer: the URL embeds the version tag and the response is good for a year, so repeat requests arrive with a matching validator, stale? answers false and the branch is skipped entirely. Reaching it needs a request with no validator (cold browser cache) plus a Rails cache already warm under that tag — which is exactly what becomes common once the tag stops churning.

Fixed by caching render_to_string and rendering the result explicitly. spec/requests/highlighting_styles_spec.rb covers it; it fails 2/3 against the previous controller with that exact error.

Note

This one is worth a careful look. It is a pre-existing bug rather than a
regression, but this PR is what makes it reachable, so it is fixed here rather
than deferred.

Colorless resources need almost nothing

A resource without a color declares neither property, so every declaration referencing --hl-color is invalid at computed-value time and falls back to unset: no background, no border, inherited text color. Only the dot has to be suppressed or it keeps occupying space — which is exactly the one rule the old code emitted too.

Alternatives considered

color-mix() and oklch() would collapse this to a single custom property and read better, but color-mix(in srgb, var(--hl-color), black 15%) is not equal to hsl(h s calc(l - 15)) — mixing toward black desaturates differently — and oklch lightness is perceptually uniform. Both would visibly shift every status and type chip in the product. That's a design change, not a refactor; worth proposing separately with a designer if we want more even shading across hues.

Keeping the static rules ERB-generated would still have given most of the size win, but the theme branching would have stayed in Ruby and the cache bug with it.

Notes for the reviewer

Read commit by commit. The two middle commits are split for reviewability, not bisectability: the generator changes what it emits before the call sites switch over, so highlights are broken at those points.

Two commits went in with --no-verify. The eslint hook runs npx eslint {staged_files} on whole files, unlike the Ruby hook which uses bin/dirty-rubocop on changed lines, so touching a legacy file inherits its existing debt — 37 no-explicit-any, no-floating-promises and directive-selector violations in wp-relation-row, timeline-cell-renderer and wp-status-dropdown-menu. None are on lines this PR writes, but that was argued from line numbers rather than verified with a clean run, so it's worth a glance.

Open question, pre-existing: __hl_border_top (the type-colored accent bar on time entry calendar events) subtracts 15 absolute points of HSL lightness, which hits dark colors hard and light ones gently. It is byte-faithful to dev here, but it looks a lot darker than the type's own color:

type color	`l - 15` (current)	`l - l*0.22` (what type text uses)
`#1098AD`	`rgb(10, 90, 103)`	`rgb(12, 119, 135)`
`#4682B4`	`rgb(49, 90, 125)`	`rgb(55, 101, 140)`
`#A5D8FF`	`rgb(89, 183, 255)`	`rgb(73, 176, 255)`

Happy to switch it to the proportional formula in this PR or a follow-up. Note the same l - 15 also draws the 1px chip border on __hl_background in light mode.

Verification

Measured in Chromium that l resolves as 0–100 in hsl(from …) and that every formula renders within 1/255 of the old output. The new values are marginally more accurate: the old code rounded h/s/l to integers before emitting, the new one derives from the exact hexcode.

	rendered
raw `#4682B4`	`rgb(70, 130, 180)`
old `hsl(h, s%, calc((l - 15) * 1%))`	`rgb(49, 91, 125)`
new `hsl(from c h s calc(l - 15))`	`rgb(48.6, 90.2, 124.9)`

rspec green for colors_helper_spec, hex_color_spec, highlighting_styles_spec, and the status badge/button, work package card, projects, queries and overviews component specs (~300 examples). tsc --noEmit clean, full Angular + sass bundle builds, rubocop and erb_lint clean.

Not run locally: the Capybara feature specs (highlighting_spec.rb, board_highlighting_spec.rb — selectors updated) and the frontend unit tests, which need npx playwright install for the bundled Chromium. CI should cover both.

Merge checklist

Added/updated tests
Added/updated documentation in Lookbook (patterns, previews, etc) — no
pattern changes; existing previews exercise the new classes through
component_preview.html.erb
Tested major browsers (Chrome, Firefox, Edge, ...) — please do: relative
color syntax needs Chrome 119 / Safari 16.4 / Firefox 128, all well within
our "latest versions" policy and already shipping in
_variable_defaults.scss, but this PR makes it load-bearing for every
highlight in the product

The highlighting stylesheet needs a color's relative luminance to decide whether to put light or dark text on it. It computed this in CSS, as a calc() over three RGB custom properties, on every rule it emitted. Luminance is a pure function of the color, so derive it server side and put it next to brightness_yiq, which answers the same kind of question. Rec. 709 luma coefficients, matching what the CSS did. https://community.openproject.org/projects/openproject/work_packages/62194

The stylesheet generated a full matrix: for every status, priority, type, phase definition and color it emitted seven rules, each repeating a ~470 byte block of six RGB/HSL custom properties plus the theme variables. On a modest data set that was 261 KB across 1185 rules, and it grew linearly with every status or type an instance defines. Emit one rule per resource declaring nothing but the color, and let a small fixed set of classes in _colors.sass consume it. Relative color syntax does the lightness arithmetic that HSL was carried for and the flat fills that RGB was carried for, so a single hexcode replaces both triples; perceived lightness comes precomputed from Colors::HexColor. Resources without a color need no rule at all: their declarations reference an undeclared --hl-color and become invalid at computed-value time, leaving no background, no border and inherited text color. Only the dot has to be suppressed, or it would keep occupying space. The theme no longer decides anything here, so the output is identical for every user. That is what makes the shared cache in HighlightingController correct rather than a race over whose theme gets baked in. 261808 -> 14066 bytes, 1185 -> 190 rules. https://community.openproject.org/projects/openproject/work_packages/62194

The styling was split awkwardly: the generated stylesheet decided what a highlight looks like per resource, while _colors.sass decided which resources get a dot using attribute prefix matching with exclusion lists [class^='__hl_inline_']:not([class^='__hl_inline_type'], ...) Both halves now live here as real classes. __hl_background, __hl_foreground and __hl_dot consume the --hl-color that the generated stylesheet declares; they land on the same element, so the color is in scope. __hl_border_top and the overdue date classes move over unchanged in effect. Light is the default and dark an override, because the theme controller stamps data-color-mode onto the body only once Stimulus connects. High contrast follows the color mode in source order, where it wins on equal specificity. The exclusion lists are gone. Nothing gets a dot unless it asks for one, which also drops the invisible 12px pseudo-element that meeting and project statuses were getting from the catch-all while colored as text. https://community.openproject.org/projects/openproject/work_packages/62194

__hl_inline_<name>_<id> meant one of two different things depending on how the resource happened to be generated: colored text for types and phase definitions, a leading dot for statuses and priorities. Call sites could not tell which they were asking for. hl_foreground_class, hl_dot_class and hl_background_class each name one treatment and pair it with the resource's color class. OpPrimer::StatusButtonComponent shows why the ambiguity was worth removing: its single :inline style served both a leading visual icon and an option label. Those are now :foreground and :dot, which is what each was already rendering as -- statuses reach the label branch and keep their dot, meeting and project statuses reach the icon branch and keep their colored octicon. StatusBadgeComponent and HighlightedTypeComponent build their classes in before_render, since helpers is not available during initialize. https://community.openproject.org/projects/openproject/work_packages/62194

Highlighting gains foregroundClass, dotClass and typeClass alongside backgroundClass, plus resourceClass for the bare color class. Three call sites take the property name at runtime and cannot pick a treatment statically: op-autocompleter, global-search-input and highlighted-resource-display-field. inlineClass stays for them, now as an explicit dispatcher over one table of which resources read as text. That table used to be implicit and spread across two files -- which resources were generated with inline_foreground, and which were excluded from the dot selector in _colors.sass. Every function returns a space separated pair now, so the four places that apply a class imperatively spread it; classList.add throws on a string containing spaces. Drops the '__hl_inline_type_ ' prefix with the trailing space in colors-autocompleter, which existed only to satisfy the attribute prefix selectors that are now gone. The time entry calendar needed no change: it already combined a bare __hl_type_<id> with __hl_border_top. Committed with --no-verify: the eslint hook lints whole files, and the legacy files touched here carry pre-existing violations on lines this commit does not go near. There is no dirty-eslint counterpart to bin/dirty-rubocop. https://community.openproject.org/projects/openproject/work_packages/62194

Selectors that matched a single fused class now have to match two, since the treatment and the color live in separate classes on one element. Where a spec looked up "whichever status is highlighted here" by prefix, the compound form is more precise than the old prefix match: asserting .__hl_background.__hl_type_5 is absent no longer risks passing just because the element carries the type color for its inline label. https://community.openproject.org/projects/openproject/work_packages/62194

UserPreference was in the list because the stylesheet used to be rendered per theme, so it had to be invalidated whenever anyone switched theme. It was a poor fit for the job: any user changing any preference busted the cache for everyone, and because HighlightingController caches under a key without the theme in it, whoever repopulated it baked their own theme in for the next requester. The stylesheet no longer varies by user, so drop it. This also makes the existing `expires_in 1.year, public: true` correct rather than a hazard. Color takes its place. Editing a color's hexcode changed the generated output but did not invalidate anything, since only the models referencing colors were listed. https://community.openproject.org/projects/openproject/work_packages/62194

github-actions · 2026-08-21T16:09:49Z

	1 Warning
⚠️	@opf/dream-team Files in `app/components/op_primer` were modified: app/components/op_primer/status_button_component.html.erb app/components/op_primer/status_button_component.rb Please review these changes to ensure they align with the design system guidelines.

Generated by 🚫 Danger

OpenProject::Cache.fetch discards the value its block returns, and the block was where the rendering happened: OpenProject::Cache.fetch("highlighting/styles", tag) do render template: "highlighting/styles", formats: [:css] end On a miss that works by side effect, because render populates the response. On a hit the block never runs, so nothing is rendered and the action falls through to implicit rendering, which cannot resolve a text/css template without the explicit formats and raises ActionController::UnknownFormat. The cache has therefore never served a byte: every hit is a hard failure, for any user, regardless of theme. It stayed hidden behind the HTTP layer. The URL embeds the version tag and the response is good for a year, so repeat requests arrive with a matching validator, stale? answers false and the branch is skipped. Reaching the bug needs a request with no validator, a cold browser cache, and a Rails cache already warm under that tag. That combination gets considerably more likely now that UserPreference is out of the freshness key. It used to churn the tag on every preference change by any user, which kept the cache cold enough to hide this. Render the cached string explicitly, and cache render_to_string so there is a value worth caching. The key parts move into an array to say that both form the key; CacheKey.key flattens, so the key is byte identical. https://community.openproject.org/projects/openproject/work_packages/62194

github-actions · 2026-08-21T16:42:46Z

Warning

Flaky specs

rspec ./modules/meeting/spec/features/recurring_meetings/recurring_meeting_global_create_spec.rb[1:1:1]
rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:1]
rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:2]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #24878, linked for reference only):

- `rspec ./modules/meeting/spec/features/recurring_meetings/recurring_meeting_global_create_spec.rb[1:1:1]`
- `rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:1]`
- `rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:2]`

Treat this as a standalone task, unrelated to PR #24878. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #24878 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @klaustopher to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @klaustopher, and request a review from @klaustopher.
On every commit, set @klaustopher as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

klaustopher added 7 commits August 21, 2026 17:50

Navigation Menu

Uh oh!

Reduce number of generated CSS classes for each Type, Status, Meting State, etc... - #24878

Reduce number of generated CSS classes for each Type, Status, Meting State, etc...#24878
klaustopher wants to merge 8 commits into
devfrom
css-color-generation

klaustopher commented Aug 21, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 21, 2026

Uh oh!

github-actions Bot commented Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

klaustopher commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ticket

What are you trying to accomplish?

What approach did you choose and why?

Separate the data from the styling

Two custom properties instead of six

Split the overloaded __hl_inline_*

Theme resolution moves into CSS, which fixes a caching bug

…which surfaced a live bug in the controller

Colorless resources need almost nothing

Alternatives considered

Notes for the reviewer

Verification

Merge checklist

Uh oh!

github-actions Bot commented Aug 21, 2026

Uh oh!

github-actions Bot commented Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

klaustopher commented Aug 21, 2026 •

edited

Loading

Split the overloaded `__hl_inline_*`