Merge pull request #1595 from basecamp/revert-1594-revert-1588-color-a11y

Color a11y

app/models/card/colored.rb

@@ -1,24 +1,7 @@
 module Card::Colored
   extend ActiveSupport::Concern
 
-  COLORS = %w[
-    var(--color-card-1)
-    var(--color-card-2)
-    var(--color-card-3)
-    var(--color-card-4)
-    var(--color-card-5)
-    var(--color-card-6)
-    var(--color-card-7)
-    var(--color-card-8)
-  ]
-  DEFAULT_COLOR = "var(--color-card-default)"
-
   def color
-    color_from_column || DEFAULT_COLOR
+    column&.color || Column::Colored::DEFAULT_COLOR
   end
-
-  private
-    def color_from_column
-      column&.color&.presence
-    end
 end

app/models/color.rb

@@ -0,0 +1,25 @@
+Color = Struct.new(:name, :value)
+
+class Color
+  class << self
+    def for_value(value)
+      COLORS.find { |it| it.value == value }
+    end
+  end
+
+  def to_s
+    value
+  end
+
+  COLORS = {
+    "Blue" => "var(--color-card-default)",
+    "Gray" => "var(--color-card-1)",
+    "Tan" => "var(--color-card-2)",
+    "Yellow" => "var(--color-card-3)",
+    "Lime" => "var(--color-card-4)",
+    "Aqua" => "var(--color-card-5)",
+    "Violet" => "var(--color-card-6)",
+    "Purple" => "var(--color-card-7)",
+    "Pink" => "var(--color-card-8)"
+  }.collect { |name, value| new(name, value) }.freeze
+end

app/models/column.rb

@@ -1,10 +1,9 @@
 class Column < ApplicationRecord
-  include Positioned
+  include Colored, Positioned
 
   belongs_to :board, touch: true
   has_many :cards, dependent: :nullify
 
-  before_validation    -> { self.color ||= Card::DEFAULT_COLOR }
   after_save_commit    -> { cards.touch_all }, if: -> { saved_change_to_name? || saved_change_to_color? }
   after_destroy_commit -> { board.cards.touch_all }
 end

app/models/column/colored.rb

@@ -0,0 +1,13 @@
+module Column::Colored
+  extend ActiveSupport::Concern
+
+  DEFAULT_COLOR = Color::COLORS.first
+
+  included do
+    before_validation -> { self[:color] ||= DEFAULT_COLOR.value }
+  end
+
+  def color
+    Color.for_value(super) || DEFAULT_COLOR
+  end
+end

app/views/boards/show/menu/_column_form.html.erb

@@ -3,11 +3,11 @@
         required: true, autocomplete: "off", pattern: ".*\\S.*", title: "Column name cannot be blank", data: { action: "focus->form#select" } %>
 
   <div class="color-picker__colors">
-    <% ([Card::DEFAULT_COLOR] + Card::COLORS).each do |color| %>
-      <label class="btn txt-small borderless" style="--btn-background: <%= color %>" title="<%= color %>">
-        <%= form.radio_button :color, color, checked: (column.color == color || (column.new_record? && color == Card::DEFAULT_COLOR)) %>
+    <% Color::COLORS.each do |color| %>
+      <label class="btn txt-small borderless" style="--btn-background: <%= color %>" title="<%= color.name %>">
+        <%= form.radio_button :color, color.value, checked: (column.color == color || (column.new_record? && color == Column::Colored::DEFAULT_COLOR)) %>
         <%= icon_tag "check", class: "checked" %>
-        <span class="for-screen-reader"><%= color %></span>
+        <span class="for-screen-reader"><%= color.name %></span>
       </label>
     <% end %>
   </div>

app/views/public/cards/show.html.erb

@@ -19,7 +19,7 @@
 <% end %>
 
 <section id="<%= dom_id(@card, :card_container) %>"
-    class="card-perma card-perma--public" data-controller="lightbox" style="--card-color: <%= @card.color %>;">
+  class="card-perma card-perma--public" data-controller="lightbox" style="--card-color: <%= @card.color %>;">
   <div class="card-perma__bg">
     <%= card_article_tag @card, class: "card" do %>
       <header class="card__header">

test/fixtures/columns.yml

@@ -1,24 +1,24 @@
 # Columns for writebook board (which has qa workflow)
 writebook_triage:
   name: Triage
-  color: "#000000"
+  color: "var(--color-card-4)"
   board: writebook
   position: 0
 
 writebook_in_progress:
   name: In progress
-  color: "#3b3633"
+  color: "var(--color-card-2)"
   board: writebook
   position: 1
 
 writebook_on_hold:
   name: On Hold
-  color: "#67695e"
+  color: "var(--color-card-4)"
   board: writebook
   position: 2
 
 writebook_review:
   name: Review
-  color: "#eb7a32"
+  color: "var(--color-card-3)"
   board: writebook
   position: 3

test/models/card/colored_test.rb

@@ -3,7 +3,7 @@ require "test_helper"
 class Card::ColoredTest < ActiveSupport::TestCase
   test "use default color if no column" do
     cards(:logo).update! column: nil
-    assert_equal Card::DEFAULT_COLOR, cards(:logo).color
+    assert_equal Column::Colored::DEFAULT_COLOR, cards(:logo).color
   end
 
   test "infer color from column" do

test/models/column/colored_test.rb

@@ -0,0 +1,16 @@
+require "test_helper"
+
+class Column::ColoredTest < ActiveSupport::TestCase
+  test "creates column with default color when color not provided" do
+    column = boards(:writebook).columns.create!(name: "New Column")
+
+    assert_equal Column::Colored::DEFAULT_COLOR, column.color
+  end
+
+  test "update the column color" do
+    columns(:writebook_triage).update!(color: "var(--color-card-3)")
+
+    assert_not_nil columns(:writebook_triage).color
+    assert_equal Color.for_value("var(--color-card-3)"), columns(:writebook_triage).color
+  end
+end

test/models/column_test.rb

@@ -1,12 +1,6 @@
 require "test_helper"
 
 class ColumnTest < ActiveSupport::TestCase
-  test "creates column with default color when color not provided" do
-    column = boards(:writebook).columns.create!(name: "New Column")
-
-    assert_equal Card::DEFAULT_COLOR, column.color
-  end
-
   test "touch all the cards when the name or color changes" do
     column = columns(:writebook_triage)