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

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.

lib/active_admin_theme/version.rb

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

package.json

@@ -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>",

test/css_check.rb

@@ -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