3.1.1: a sort marker you can read, and guards that actually guard

A review of 3.1.0 found that several of the checks added alongside it were
quiet rather than wrong, and that one visual change lost information. Six
fixes, each reproduced before and after.

## Unsorted and descending looked the same

ActiveAdmin's sprite had three glyphs: a double arrow for sortable-but-unsorted,
up for ascending, down for descending. The replacement drew the same down
triangle for unsorted and descending, separated only by 0.6 against 1.0
opacity on an 8x4 shape — so a freshly loaded index showed every heading
pointing down. Three shapes again, drawn as masks in currentColor like the
theme switch icons, so they still follow the heading colour into dark mode.

## The type guards for the new palette were never tested

Removing all five $skinStatusTag*Color entries from the colour guard left
`rake css` green: the one BAD fixture passed because `none` crashes mix()
inside the theme, not because the guard fired. A rejection whose message
names gem internals is exactly what the guards exist to replace. The BAD
check now requires the message to name the variable the fixture set, and
there is a fixture per variable rather than one for five. With the guards
deleted it fails five times over, naming each.

## An example that failed the contract beside it

GOOD["status tags recoloured"] paired $skinStatusTagTextColor: #f5f5f5 with
fills left at their defaults, giving 4.17 to 4.19:1 on four of the five —
under the 4.5:1 the guard two screens away enforces. The contrast guard only
ever compiles the defaults, so it could not see its own neighbour.

## Checks that hid each other

The contrast and README checks sat behind `if failures.empty?`, and the first
aborted before the second ran. A run could report one problem while holding
three, and each fix revealed the next. They are collected and reported
together now. For the same reason a single translucent fill no longer returns
early and hides the other four, and the label is no longer reported as a tag
named "label".

## Two lists that drift

TAG_COLOURS was five names typed out by hand — the drift DECLARED_ROWS was
deleted for. It is read from the stylesheet, so a sixth tag colour cannot be
silently exempt. And the !default scanner had no comment awareness, so a note
like `// was: $skinStatusTagOkColor: #8daa92!default;` failed the build
twice over, blaming the live declaration.

## Smaller

* ActiveAdmin's sprite is cleared from every anchor in a sortable heading, not
  only the one the theme marks. An application's own link kept the PNG and the
  13px indent it needs.
* $skinTableHeaderTextColorDark derives from $skinTextColorDark, matching its
  light twin and the sentence the README already carried. It was a literal
  that happens to equal it today.

Author: Fivell

README.md

@@ -35,7 +35,7 @@ Or add manually to `package.json`:
 
 ```
 "dependencies": {
-  "@activeadmin-plugins/active_admin_theme": "^3.1.0"
+  "@activeadmin-plugins/active_admin_theme": "^3.1.1"
 }
 ```
 and execute:
@@ -273,7 +273,7 @@ Three things changed shape and are worth knowing if you already 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` | `$skinTextColor` / `#dde2e8` | index-table column header text; the body text colour, so headings read as strongly as the rows |
+| `$skinTableHeaderTextColor` / `$skinTableHeaderTextColorDark` | `$skinTextColor` / `$skinTextColorDark` | 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` |

app/assets/stylesheets/wigu/active_admin_theme.scss

@@ -130,7 +130,7 @@ $skinInactiveTabTextColor: #5e6469!default;
 $skinInactiveTabTextColorDark: #b0b8c2!default;
 // Index-table column header text, one colour for sortable and plain headers.
 $skinTableHeaderTextColor: $skinTextColor!default;
-$skinTableHeaderTextColorDark: #dde2e8!default;
+$skinTableHeaderTextColorDark: $skinTextColorDark!default;
 
 // 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
@@ -233,6 +233,15 @@ html[data-theme="dark"]  { @include aa-dark-palette; }
   html[data-theme="dark"] { @content; }
 }
 
+// Маркеры сортировки. Три разные формы, а не одна с разной прозрачностью:
+// у спрайта ActiveAdmin «не отсортировано» — двойная стрелка, и если заменить
+// её тем же треугольником, что у «по убыванию», состояния становятся
+// неразличимы на свежей странице. Маской, как иконки переключателя, — чтобы
+// красились currentColor и шли за цветом заголовка в обоих режимах.
+$sort-icon-none: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 9 12'%3E%3Cpath d='M4.5 0 9 5H0z'/%3E%3Cpath d='M4.5 12 0 7h9z'/%3E%3C/svg%3E")!default;
+$sort-icon-asc: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 9 12'%3E%3Cpath d='M4.5 2 9 8H0z'/%3E%3C/svg%3E")!default;
+$sort-icon-desc: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 9 12'%3E%3Cpath d='M4.5 10 0 4h9z'/%3E%3C/svg%3E")!default;
+
 // Иконки переключателя тем. Инлайном, потому что гем не возит картинок, и
 // маской, а не цветным SVG: маска красится currentColor и сама идёт за
 // $skinMenuTextColor. Половинка круга — auto, солнце — light, месяц — dark.
@@ -905,27 +914,27 @@ body.active_admin {
     // 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="] {
+    // The sprite goes from every anchor in a sortable heading, because
+    // ActiveAdmin sets it on every one — an application link in there would
+    // otherwise keep the low-res PNG and the 13px indent it needs.
+    th.sortable a {
       padding-left: 0;
       background-image: none;
+    }
+    // The marker, though, only on ActiveAdmin's own heading link, which always
+    // carries the order parameter: an application's link is its own business.
+    th.sortable > a[href*="order="] {
 
       &:after {
         content: "";
         display: inline-block;
+        width: 9px;
+        height: 12px;
         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;
+        background-color: currentColor;
+        -webkit-mask: #{$sort-icon-none} center / contain no-repeat;
+        mask: #{$sort-icon-none} center / contain no-repeat;
         // 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
@@ -934,13 +943,15 @@ body.active_admin {
       }
     }
     th.sorted-asc > a[href*="order="]:after {
-      border-top-width: 0;
-      border-bottom-width: 4px;
-      border-top-color: transparent;
-      border-bottom-color: currentColor;
+      -webkit-mask-image: $sort-icon-asc;
+      mask-image: $sort-icon-asc;
+      opacity: 1;
+    }
+    th.sorted-desc > a[href*="order="]:after {
+      -webkit-mask-image: $sort-icon-desc;
+      mask-image: $sort-icon-desc;
       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.

img/dark.png

@@ -1,3 +1,3 @@
 module ActiveAdminTheme
-  VERSION = "3.1.0"
+  VERSION = "3.1.1"
 end

img/inputs.png

@@ -1,6 +1,6 @@
 {
   "name": "@activeadmin-plugins/active_admin_theme",
-  "version": "3.1.0",
+  "version": "3.1.1",
   "description": "Flat design for ActiveAdmin",
   "main": "src/active_admin_theme.scss",
   "author": "Igor Fedoronchuk <igor.f@didww.com>",

img/light.png

@@ -29,7 +29,14 @@ 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;',
+    # Every colour here must clear LABEL_MINIMUM against the label, because the
+    # contrast guard measures the shipped palette: an example that fails the
+    # contract documents the wrong thing.
+    "status tags recoloured" => '$skinStatusTagOkColor: #1f7a3a;
+                                 $skinStatusTagNeutralColor: #55595f;
+                                 $skinStatusTagNoticeColor: #2f62b4;
+                                 $skinStatusTagWarnColor: #875c12;
+                                 $skinStatusTagErrorColor: #b03a2e;',
   }.freeze
 
   # Wrong-typed overrides. All of these are legal SassScript, so without the
@@ -49,14 +56,19 @@ module CssCheck
     "$skinPanelHeaderColor as a length"   => '$skinPanelHeaderColor: 10px;',
     "$skinStatusTagTextColor: none"       => '$skinStatusTagTextColor: none;',
     "$skinStatusTagOkColor: none"         => '$skinStatusTagOkColor: none;',
+    "$skinStatusTagNeutralColor: none"    => '$skinStatusTagNeutralColor: none;',
+    "$skinStatusTagNoticeColor: none"     => '$skinStatusTagNoticeColor: none;',
+    "$skinStatusTagWarnColor: none"       => '$skinStatusTagWarnColor: none;',
+    "$skinStatusTagErrorColor: none"      => '$skinStatusTagErrorColor: none;',
+    "$skinStatusTagTextColor as a length" => '$skinStatusTagTextColor: 10px;',
   }.freeze
 
   # The variables table in the README is the public contract people configure
   # against, and it had drifted from the declarations in 30 of 52 rows after the
   # defaults moved to yeti-web's configuration. Nothing noticed, because nothing
   # was comparing them.
   def self.readme_table_matches_declarations
-    scss = File.read(File.join(STYLESHEETS, "wigu/active_admin_theme.scss"))
+    scss = strip_comments(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|
@@ -156,16 +168,29 @@ def self.contrast(one, two)
   # 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
+  # Read from the stylesheet, not typed out here. A hand-kept list is the same
+  # drift this file removed when DECLARED_ROWS went: add a sixth tag colour and
+  # it would be silently exempt from the contrast check for ever.
+  # Sass ignores a commented-out declaration; this file used to count one, and
+  # with the duplicate and mismatch checks in place that turned a note like
+  # `// was: $skinStatusTagOkColor: #8daa92!default;` into a red build blaming
+  # the live declaration.
+  def self.strip_comments(scss)
+    scss.gsub(%r{/\*.*?\*/}m, "").gsub(%r{//[^\n]*}, "")
+  end
+
+  def self.tag_colours
+    @tag_colours ||= begin
+      scss = strip_comments(File.read(File.join(STYLESHEETS, "wigu/active_admin_theme.scss")))
+      names = scss.scan(/\$skinStatusTag([A-Za-z0-9]+)Color\s*:[^;]*!default/).flatten
+      names.reject! { |name| name == "Text" }
+      raise "css_check: no $skinStatusTag*Color declarations found" if names.empty?
+      names.uniq.to_h { |name| [name.downcase, "$skinStatusTag#{name}Color"] }
+    end
+  end
 
   def self.status_tag_palette
-    probe = TAG_COLOURS.merge("label" => "$skinStatusTagTextColor").map do |name, variable|
+    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
@@ -177,7 +202,7 @@ def self.status_tag_palette
     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
+    missing = (tag_colours.keys + ["label"]) - found.keys
     raise "css_check: the status tag probe returned nothing for #{missing.join(", ")}" unless missing.empty?
     found
   end
@@ -191,20 +216,26 @@ def self.status_tag_palette
   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
+    problems = []
 
-    TAG_COLOURS.keys.filter_map do |name|
-      fill = palette.fetch(name)[:rgb]
-      ratio = contrast(fill, label[:rgb])
+    # The label is not a tag, and reporting it as one sent a reader looking for
+    # a `label` status class that does not exist.
+    problems << "$skinStatusTagTextColor is translucent, so no tag ratio can be measured" if label[:alpha] < 1
+
+    tag_colours.each_key do |name|
+      fill = palette.fetch(name)
+      # Reported, not skipped, and without abandoning the other four: a single
+      # translucent fill used to return early and hide every failure behind it.
+      if fill[:alpha] < 1 || label[:alpha] < 1
+        problems << "status tag #{name}: translucent, so the label ratio cannot be measured"
+        next
+      end
+      ratio = contrast(fill[:rgb], 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}"
+      problems << "status tag #{name}: label #{hex(label[:rgb])} on #{hex(fill[:rgb])} is " \
+                  "#{format("%.3f", ratio)}:1, under #{LABEL_MINIMUM}"
     end
+    problems
   end
 
   def self.hex(rgb)
@@ -239,8 +270,16 @@ def self.run
     BAD.each do |name, overrides|
       compile(overrides)
       failures << "#{name}: should be rejected with @error, but compiled silently"
-    rescue SassC::SyntaxError
-      # expected — the theme's type guards caught it
+    rescue SassC::SyntaxError => e
+      # Rejected is not enough: the point of the guards is that the message
+      # names the variable the host set. Without this, a fixture passes when
+      # the wrong value merely crashes something downstream — `none` reaching
+      # mix() inside the theme reads as a rejection while naming gem internals,
+      # and the guard it was written to prove can be deleted unnoticed.
+      variable = name[/\$skin[A-Za-z0-9]+/]
+      next if variable.nil? || e.message.include?(variable)
+      failures << "#{name}: rejected, but the message does not name #{variable} — " \
+                  "#{e.message.lines.first.to_s.strip}"
     end
 
     # The header menu's text colours must follow the variables. A hard-coded
@@ -269,25 +308,20 @@ def self.run
                   "#{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
+    # One list, reported together. Behind `if failures.empty?` these two were
+    # invisible whenever anything else failed, and the first of them aborted
+    # before the second ran — so a run could report one problem while holding
+    # three, and each fix revealed the next.
+    failures.concat(status_tag_labels_are_readable)
+    failures.concat(readme_table_matches_declarations)
 
-      drift = readme_table_matches_declarations
-      unless drift.empty?
-        drift.each { |line| warn "css_check: #{line}" }
-        abort "css_check: the README variables table is out of sync in #{drift.size} place(s)"
-      end
-
-      puts "css_check: #{GOOD.size} overrides compile clean, #{BAD.size} bad ones rejected, " \
-           "README table matches #{compared_declarations} declarations"
-    else
+    unless failures.empty?
       failures.each { |failure| warn "css_check: #{failure}" }
       abort "css_check: #{failures.size} problem(s)"
     end
+
+    puts "css_check: #{GOOD.size} overrides compile clean, #{BAD.size} bad ones rejected, " \
+         "README table matches #{compared_declarations} declarations"
   end
 end