diff --git a/README.md b/README.md index 60a5a10..564068c 100644 --- a/README.md +++ b/README.md @@ -262,8 +262,13 @@ set variables: | `$skinTabInactiveColor` / `$skinTabInactiveColorDark` | `#f7f9fb` / `#161a1e` | inactive tab fill | | `$skinActiveTabTextColor` / `$skinActiveTabTextColorDark` | `$skinMainSecondColor` / `#7cc0ec` | selected tab label | | `$skinInactiveTabTextColor` / `$skinInactiveTabTextColorDark` | `#5e6469` / `#b0b8c2` | inactive tab label | -| `$skinTableHeaderTextColor` / `$skinTableHeaderTextColorDark` | `#5e6469` / `#dde2e8` | index-table column header text | -| `$skinStatusTagTextColor` | `#ffffff` | status tag label; `#000000` passes WCAG AA on every fill | +| `$skinTableHeaderTextColor` / `$skinTableHeaderTextColorDark` | `$skinTextColor` / `#dde2e8` | index-table column header text; the body text colour, so headings read as strongly as the rows | +| `$skinStatusTagTextColor` | `#ffffff` | label inside a filled status tag; `empty` / `unknown` / `none` have no fill and keep `$skinTextMutedColor` | +| `$skinStatusTagNeutralColor` | `#707681` | unclassified tags: `No`, protocol tags | +| `$skinStatusTagOkColor` | `#5e7e63` | `ok` `published` `complete` `completed` `green` `yes` | +| `$skinStatusTagNoticeColor` | `#3874d2` | `notice` `blue` | +| `$skinStatusTagWarnColor` | `#9e6c15` | `warn` `warning` `orange` | +| `$skinStatusTagErrorColor` | `#ce483b` | `error` `errored` `red` | | `$skinTabPaddingY` | `8px` | tab height | | `$skinTabPaddingX` | `15px` | tab label horizontal padding (text → border) | diff --git a/app/assets/stylesheets/wigu/active_admin_theme.scss b/app/assets/stylesheets/wigu/active_admin_theme.scss index e0220f8..0352f71 100644 --- a/app/assets/stylesheets/wigu/active_admin_theme.scss +++ b/app/assets/stylesheets/wigu/active_admin_theme.scss @@ -129,10 +129,20 @@ $skinActiveTabTextColorDark: #7cc0ec!default; $skinInactiveTabTextColor: #5e6469!default; $skinInactiveTabTextColorDark: #b0b8c2!default; // Index-table column header text, one colour for sortable and plain headers. -$skinTableHeaderTextColor: #5e6469!default; +$skinTableHeaderTextColor: $skinTextColor!default; $skinTableHeaderTextColorDark: #dde2e8!default; -// Status tag label, the same on every filled tag in both modes. + +// Status tags. The label is the same on every filled tag in both modes. The +// fills below are dark enough to carry a white one: every one of the five is +// at least 4.53 against it. The mid-tone fills this palette replaces ran from +// 2.35 to 3.78, under the 4.5 WCAG AA asks of text this small, which is why +// they moved rather than the label. $skinStatusTagTextColor: #ffffff!default; +$skinStatusTagNeutralColor: #707681!default; // "No", protocol tags, anything unclassified +$skinStatusTagOkColor: #5e7e63!default; // ok / published / complete / green / yes +$skinStatusTagNoticeColor: #3874d2!default; // notice / blue +$skinStatusTagWarnColor: #9e6c15!default; // warn / warning / orange +$skinStatusTagErrorColor: #ce483b!default; // error / errored / red //DARK-MODE PALETTE---------------------------------------------------------------------------------------------------// // Semantic CSS custom properties for runtime light/dark switching. Light values @@ -304,7 +314,12 @@ $theme-icon-dark: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/s skinInactiveTabTextColorDark: $skinInactiveTabTextColorDark, skinTableHeaderTextColor: $skinTableHeaderTextColor, skinTableHeaderTextColorDark: $skinTableHeaderTextColorDark, - skinStatusTagTextColor: $skinStatusTagTextColor + skinStatusTagTextColor: $skinStatusTagTextColor, + skinStatusTagNeutralColor: $skinStatusTagNeutralColor, + skinStatusTagOkColor: $skinStatusTagOkColor, + skinStatusTagNoticeColor: $skinStatusTagNoticeColor, + skinStatusTagWarnColor: $skinStatusTagWarnColor, + skinStatusTagErrorColor: $skinStatusTagErrorColor ) { @if type-of($value) != color { @error "$#{$name} must be a color (use `transparent`, not `none`), got `#{$value}`."; @@ -879,6 +894,53 @@ body.active_admin { font-weight: bold!important; } } + // ActiveAdmin puts the sort arrow inside the heading link, on the left, as a + // background image cleared with `padding-left: 13px`. That indents the label + // of every sortable column by 13px while the data below starts at the cell's + // own padding, so a heading never lines up with its column. The link is also + // `display: block`, so moving the image to the right edge would park the + // arrow at the far side of the column instead of beside the text. + // + // Drawn as a pseudo-element instead: it follows the label immediately, the + // link keeps its full width so the whole cell stays clickable, and the + // marker takes currentColor — the stock sprite is a fixed grey PNG that + // cannot follow the text into dark mode. + // `a[href*="order="]`, not every anchor in the header: an application can + // put its own link in there — yeti-web adds a persistent-sort toggle — and + // it would otherwise get a sort marker of its own. ActiveAdmin's heading + // link always carries the order parameter. + th.sortable > a[href*="order="] { + padding-left: 0; + background-image: none; + + &:after { + content: ""; + display: inline-block; + margin-left: 6px; + vertical-align: middle; + // The unused side has no width rather than a transparent one, so the + // box is exactly as tall as the triangle in it. Keeping all four sides + // and nudging with a margin instead puts the two states at different + // heights, because `vertical-align: middle` centres the box and the + // visible half then sits off-centre within it. + border: 4px solid transparent; + border-bottom-width: 0; + border-top-color: currentColor; + // 0.6, not lower: the marker is the only thing separating a sortable + // heading from a plain one, so WCAG 1.4.11 asks 3:1 of it. Against the + // header fill it gives 3.40 light and 4.22 dark; at 0.4 it was 2.13 + // and 2.73. + opacity: 0.6; + } + } + th.sorted-asc > a[href*="order="]:after { + border-top-width: 0; + border-bottom-width: 4px; + border-top-color: transparent; + border-bottom-color: currentColor; + opacity: 1; + } + th.sorted-desc > a[href*="order="]:after { opacity: 1; } // Right edge = a single 1px line on the last-column cells (header th, body // td, footer cells) coloured like the table border, since the table itself // no longer draws a right border. @@ -1418,9 +1480,8 @@ input[type='radio'] { @mixin status-tag-colors($c) { background: $c; border-color: mix($c, #000000, 84%); // = darken($c) — outline in the fill colour - // The five fills below are mid-tone: white lands between 2.35 and 3.78 - // against them, under the 4.5 WCAG AA asks for text this small. Set - // $skinStatusTagTextColor: #000000 for at least 5.56 on every one of them. + // The fills are dark enough to carry this label: every one of the five is at + // least 4.53 against white. Lighten one and the label needs to go with it. color: $skinStatusTagTextColor; } @@ -1453,11 +1514,11 @@ input[type='radio'] { border-radius: 2px; border: 1px solid; - @include status-tag-colors(#8a909a); // neutral default (e.g. "No", protocol tags) - &.ok, &.published, &.complete, &.completed, &.green, &.yes { @include status-tag-colors(#8daa92); } - &.notice, &.blue { @include status-tag-colors(#6090db); } - &.warn, &.warning, &.orange { @include status-tag-colors(#e29b20); } - &.error, &.errored, &.red { @include status-tag-colors(#d45f53); } + @include status-tag-colors($skinStatusTagNeutralColor); + &.ok, &.published, &.complete, &.completed, &.green, &.yes { @include status-tag-colors($skinStatusTagOkColor); } + &.notice, &.blue { @include status-tag-colors($skinStatusTagNoticeColor); } + &.warn, &.warning, &.orange { @include status-tag-colors($skinStatusTagWarnColor); } + &.error, &.errored, &.red { @include status-tag-colors($skinStatusTagErrorColor); } &.empty, &.unknown, &.none { background: none; border: 0; // placeholder/empty tags stay unobtrusive (no outline) diff --git a/img/dark.png b/img/dark.png index dde249c..f282625 100644 Binary files a/img/dark.png and b/img/dark.png differ diff --git a/img/inputs.png b/img/inputs.png index a91cd09..efae1e2 100644 Binary files a/img/inputs.png and b/img/inputs.png differ diff --git a/img/light.png b/img/light.png index 7a508da..8b07949 100644 Binary files a/img/light.png and b/img/light.png differ diff --git a/img/switch.png b/img/switch.png index 9d5fedf..4d940d3 100644 Binary files a/img/switch.png and b/img/switch.png differ diff --git a/test/css_check.rb b/test/css_check.rb index 0c1bd93..4cb4dd1 100644 --- a/test/css_check.rb +++ b/test/css_check.rb @@ -29,6 +29,7 @@ module CssCheck "black status tag labels" => '$skinStatusTagTextColor: #000000;', "repainted palette" => '$skinPageBgColor: #fafafa; $skinSurfaceColor: #ffffff; $skinTextColor: #202020; $skinLinkColor: #0b5;', + "status tags recoloured" => '$skinStatusTagOkColor: #1f7a3a; $skinStatusTagTextColor: #f5f5f5;', }.freeze # Wrong-typed overrides. All of these are legal SassScript, so without the @@ -47,6 +48,7 @@ module CssCheck "$skinLinkColorDark: none" => '$skinLinkColorDark: none;', "$skinPanelHeaderColor as a length" => '$skinPanelHeaderColor: 10px;', "$skinStatusTagTextColor: none" => '$skinStatusTagTextColor: none;', + "$skinStatusTagOkColor: none" => '$skinStatusTagOkColor: none;', }.freeze # The variables table in the README is the public contract people configure @@ -56,7 +58,11 @@ module CssCheck def self.readme_table_matches_declarations scss = File.read(File.join(STYLESHEETS, "wigu/active_admin_theme.scss")) declared = {} + duplicates = [] scss.scan(/(\$skin[A-Za-z0-9]+)\s*:\s*(.+?)!default/) do |name, value| + # Sass keeps the first !default and ignores the rest, so a second + # declaration is dead code that drifts from the live one in silence. + duplicates << name if declared.key?(name) # `if($x == null, 4.5px, $x)` documents as the fallback it falls back to. declared[name] ||= value.strip.sub(/\Aif\(\$\w+ == null, (.+?), \$\w+\)\z/, '\\1') end @@ -64,32 +70,159 @@ def self.readme_table_matches_declarations readme = File.read(File.expand_path("../README.md", __dir__)) rows = readme.scan(/^\|\s*`(\$skin[A-Za-z0-9]+)`(?:\s*\/\s*`(\$skin[A-Za-z0-9]+)`)?\s*\|\s*([^|]*?)\s*\|/) - rows.flat_map do |light, dark, documented| - parts = documented.split("/").map { |part| part.strip.delete("`") } - pairs = [[light, parts[0]]] - pairs << [dark, parts[1]] if dark - pairs.filter_map do |name, value| - next if value.nil? || value.empty? - actual = declared[name] - next if actual && actual.casecmp?(value) - "#{name}: README says `#{value}`, the stylesheet declares `#{actual || "nothing"}`" + documented = {} + listed_twice = [] + blank = [] + rows.each do |light, dark, values| + parts = values.split("/").map { |part| part.strip.delete("`") } + [[light, parts[0]], [dark, parts[1]]].each do |name, value| + next if name.nil? + # Every row counts towards duplicate detection, blank or not. Skipping + # blanks here instead would let a name carry one blank row and one good + # row: the good row satisfies `documented`, the blank one never reaches + # `listed_twice`, and the contradiction passes. + listed_twice << name if documented.key?(name) || blank.include?(name) + # A cell with no value documents nothing, so it does not go into + # `documented` — otherwise the name would be exempt from the + # undocumented check below while the mismatch check skipped it for + # having no value, and it would pass on both sides. + if value.nil? || value.empty? + blank << name + else + documented[name] = value + end end end + @compared_declarations = declared.size + + mismatched = documented.filter_map do |name, value| + actual = declared[name] + next if actual && actual.casecmp?(value) + "#{name}: README says `#{value}`, the stylesheet declares `#{actual || "nothing"}`" + end + + # Both directions: comparing only the documented names would let a new + # variable ship undocumented while the success line still claimed the table + # matched every declaration. Absent is measured against every row, blank + # ones included — a name with a blank row is listed, just not documented, + # and telling its author it is absent sends them looking for a row that is + # already there. + undocumented = (declared.keys - documented.keys - blank).map do |name| + "#{name}: declared in the stylesheet, absent from the README table" + end + duplicated = duplicates.uniq.map do |name| + "#{name}: declared more than once; Sass keeps the first !default and drops the rest" + end + # The last row wins when a name is listed twice, so the table can agree with + # the stylesheet while a reader meets the stale row first. + redocumented = listed_twice.uniq.map do |name| + "#{name}: listed more than once in the README table" + end + undefaulted = blank.uniq.map do |name| + "#{name}: listed in the README table with no default" + end + + mismatched + undocumented + duplicated + redocumented + undefaulted end - DECLARED_ROWS = 53 + + # Reported in the success line. Counted from the declarations themselves, so + # it cannot drift the way a hand-maintained constant does. + def self.compared_declarations + @compared_declarations || 0 + end + + # The status tag label is the one thing in this stylesheet that has been got + # wrong twice: once by shipping white on a mid-tone fill, once by darkening + # the fill and leaving the label. Nothing was checking it, so nothing caught + # either. 4.5:1 is what WCAG AA asks of text this size. + LABEL_MINIMUM = 4.5 + + def self.relative_luminance(rgb) + linear = rgb.map do |channel| + value = channel / 255.0 + value <= 0.03928 ? value / 12.92 : ((value + 0.055) / 1.055)**2.4 + end + 0.2126 * linear[0] + 0.7152 * linear[1] + 0.0722 * linear[2] + end + + def self.contrast(one, two) + lighter, darker = [relative_luminance(one), relative_luminance(two)].minmax.reverse + (lighter + 0.05) / (darker + 0.05) + end + + # Sass resolves the colour, not a regular expression over its output. Hex, + # rgb(), hsl() and named colours are all legal here and the type guard accepts + # every one; sassc normalises most of them to hex but emits names as names, so + # a regex over the stylesheet silently skipped `darkseagreen` and crashed on a + # four-digit hex. Asking Sass for the channels removes the question. + TAG_COLOURS = { + "neutral" => "$skinStatusTagNeutralColor", + "ok" => "$skinStatusTagOkColor", + "notice" => "$skinStatusTagNoticeColor", + "warn" => "$skinStatusTagWarnColor", + "error" => "$skinStatusTagErrorColor", + }.freeze + + def self.status_tag_palette + probe = TAG_COLOURS.merge("label" => "$skinStatusTagTextColor").map do |name, variable| + ".css-check-#{name} { r: red(#{variable}); g: green(#{variable}); " \ + "b: blue(#{variable}); a: alpha(#{variable}); }" + end + css = compile("", probe.join("\n")) + # Fractional channels, because red() on an hsl() colour does not return a + # whole number and an integer-only pattern silently matched nothing. + channels = css.scan(%r{\.css-check-(\w+)\s*\{\s*r:\s*([\d.]+);\s*g:\s*([\d.]+);\s* + b:\s*([\d.]+);\s*a:\s*([\d.]+);?\s*\}}x) + found = channels.to_h do |name, r, g, b, a| + [name, { rgb: [r, g, b].map { |v| v.to_f.round }, alpha: a.to_f }] + end + missing = (TAG_COLOURS.keys + ["label"]) - found.keys + raise "css_check: the status tag probe returned nothing for #{missing.join(", ")}" unless missing.empty? + found + end + + # Alpha is read, not dropped. A translucent colour has no measurable ratio — + # it would be against whatever shows through, which this stylesheet does not + # know — and dropping it would score `transparent` as black, so a transparent + # fill under a white label would pass at 21:1. The shipped palette has no + # reason to be translucent, so say so rather than skip it: a check that goes + # quiet is the failure this guard exists to avoid. + def self.status_tag_labels_are_readable + palette = status_tag_palette + label = palette.fetch("label") + translucent = palette.select { |_, colour| colour[:alpha] < 1 }.keys + unless translucent.empty? + return translucent.map do |name| + "status tag #{name}: translucent, so the label ratio cannot be measured" + end + end + + TAG_COLOURS.keys.filter_map do |name| + fill = palette.fetch(name)[:rgb] + ratio = contrast(fill, label[:rgb]) + next if ratio >= LABEL_MINIMUM + "status tag #{name}: label #{hex(label[:rgb])} on #{hex(fill)} is " \ + "#{format("%.2f", ratio)}:1, under #{LABEL_MINIMUM}" + end + end + + def self.hex(rgb) + format("#%02x%02x%02x", *rgb) + end def self.load_paths activeadmin = Gem::Specification.find_by_name("activeadmin").gem_dir [File.join(activeadmin, "app/assets/stylesheets"), STYLESHEETS] end - def self.compile(overrides) + def self.compile(overrides, appended = nil) source = <<~SCSS @import "active_admin/mixins"; #{overrides} @import "active_admin/base"; @import "#{THEME}"; + #{appended} SCSS SassC::Engine.new(source, load_paths: load_paths, style: :expanded).render end @@ -124,13 +257,25 @@ def self.run # A block-level item (flex, block, grid) breaks that row and stacks the # username, theme switch and logout on top of each other. utility = compile(GOOD["defaults"]).scan(/^[^{}]*#utility_nav\s*>\s*li[^{}\s,]*\s*\{[^}]*\}/m) - blocky = utility.select { |rule| rule =~ /^\s*display:\s*(?:flex|block|grid)\s*;/ } + # The body, not the start of a line: sassc happens to put the first + # declaration on its own line, so matching from `^` only works while + # `display` is written first in the stylesheet. Reorder the two lines in the + # source and the guard goes blind. + blocky = utility.select do |rule| + rule[/\{(.*)\}/m, 1].to_s.split(";").any? { |d| d.strip =~ /\Adisplay:\s*(?:flex|block|grid)\z/ } + end unless blocky.empty? failures << "utility nav: #{blocky.size} item rule(s) make the li block-level and break the inline row: " \ "#{blocky.map { |rule| rule[/\A[^{]*/].strip }.join(", ")}" end if failures.empty? + unreadable = status_tag_labels_are_readable + unless unreadable.empty? + unreadable.each { |line| warn "css_check: #{line}" } + abort "css_check: #{unreadable.size} status tag(s) fail the label contrast minimum" + end + drift = readme_table_matches_declarations unless drift.empty? drift.each { |line| warn "css_check: #{line}" } @@ -138,7 +283,7 @@ def self.run end puts "css_check: #{GOOD.size} overrides compile clean, #{BAD.size} bad ones rejected, " \ - "README table matches #{DECLARED_ROWS} declarations" + "README table matches #{compared_declarations} declarations" else failures.each { |failure| warn "css_check: #{failure}" } abort "css_check: #{failures.size} problem(s)"