Highlight erroneous fields by adding a class to them

The default rails scheme for highlighting errors is to wrap the
field in a div, but that changes the structure of the page and can
change the meaning of CSS rules applied to the fields.

As an alternative we now apply a class to the fields, and use that
in the CSS to apply a highlight.

diff --git a/app/assets/stylesheets/common.css.scss b/app/assets/stylesheets/common.css.scss
index d73d1000ac189b381dbeca288d3b8871376e1e17..d0aa003d776023588c772ea35deb7f291dc7227f 100644 (file)
--- a/app/assets/stylesheets/common.css.scss
+++ b/app/assets/stylesheets/common.css.scss
@@ -1725,12 +1725,6 @@ header .search_form {
 
 /* Rules for highlighting fields with rails validation errors */
 
-.field_with_errors {
-  padding: 2px;
-  background-color: #ff7070;
-  display: inline-block;
-}
-
 .formError {
   display: inline-block;
   padding: 5px 10px;
@@ -1843,6 +1837,10 @@ textarea {
   padding: 2px 5px;
   margin: 0;
   width: 200px;
+
+  &.field_with_errors {
+    border: 2px solid #ff7070;
+  }
 }
 
 textarea {

diff --git a/config/initializers/field_error.rb b/config/initializers/field_error.rb
new file mode 100644 (file)
index 0000000..41f2c2d
--- /dev/null
+++ b/config/initializers/field_error.rb
@@ -0,0 +1,9 @@
+ActionView::Base.field_error_proc = Proc.new do |html_tag, instance|
+  class_attr_index = html_tag.index 'class="'
+
+  if class_attr_index
+    html_tag.insert class_attr_index+7, 'field_with_errors '
+  else
+    html_tag.insert html_tag.index(/\/?>/), ' class="field_with_errors"'
+  end
+end

diff --git a/test/functional/user_controller_test.rb b/test/functional/user_controller_test.rb
index d4121c1bd495765a478d7cdee90c5258eb115f31..3a9c93238c0c7cfa66895fcf8dcf070ea2db3000 100644 (file)
--- a/test/functional/user_controller_test.rb
+++ b/test/functional/user_controller_test.rb
@@ -252,7 +252,7 @@ class UserControllerTest < ActionController::TestCase
 
     assert_response :success
     assert_template 'new'
-    assert_select "form > fieldset > div.form-row > div.field_with_errors > input#user_email"
+    assert_select "form > fieldset > div.form-row > input.field_with_errors#user_email"
   end
 
   def test_user_create_submit_duplicate_email_uppercase
@@ -267,7 +267,7 @@ class UserControllerTest < ActionController::TestCase
 
     assert_response :success
     assert_template 'new'
-    assert_select "form > fieldset > div.form-row > div.field_with_errors > input#user_email"
+    assert_select "form > fieldset > div.form-row > input.field_with_errors#user_email"
   end
     
   def test_user_create_submit_duplicate_name
@@ -282,7 +282,7 @@ class UserControllerTest < ActionController::TestCase
 
     assert_response :success
     assert_template 'new'
-    assert_select "form > fieldset > div.form-row > div.field_with_errors > input#user_display_name"
+    assert_select "form > fieldset > div.form-row > input.field_with_errors#user_display_name"
   end
   
   def test_user_create_submit_duplicate_name_uppercase
@@ -297,7 +297,7 @@ class UserControllerTest < ActionController::TestCase
 
     assert_response :success
     assert_template 'new'
-    assert_select "form > fieldset > div.form-row > div.field_with_errors > input#user_display_name"
+    assert_select "form > fieldset > div.form-row > input.field_with_errors#user_display_name"
   end
 
   def test_user_save_referer_params
@@ -478,7 +478,7 @@ class UserControllerTest < ActionController::TestCase
     assert_template :account
     assert_select ".notice", false
     assert_select "div#errorExplanation"
-    assert_select "form#accountForm > fieldset > div.form-row > div.field_with_errors > input#user_display_name"
+    assert_select "form#accountForm > fieldset > div.form-row > input.field_with_errors#user_display_name"
 
     # Changing name to one that exists should fail, regardless of case
     new_attributes = user.attributes.dup.merge(:display_name => users(:public_user).display_name.upcase)
@@ -487,7 +487,7 @@ class UserControllerTest < ActionController::TestCase
     assert_template :account
     assert_select ".notice", false
     assert_select "div#errorExplanation"
-    assert_select "form#accountForm > fieldset > div.form-row > div.field_with_errors > input#user_display_name"
+    assert_select "form#accountForm > fieldset > div.form-row > input.field_with_errors#user_display_name"
 
     # Changing name to one that doesn't exist should work
     new_attributes = user.attributes.dup.merge(:display_name => "new tester")
@@ -508,7 +508,7 @@ class UserControllerTest < ActionController::TestCase
     assert_template :account
     assert_select ".notice", false
     assert_select "div#errorExplanation"
-    assert_select "form#accountForm > fieldset > div.form-row > div.field_with_errors > input#user_new_email"
+    assert_select "form#accountForm > fieldset > div.form-row > input.field_with_errors#user_new_email"
 
     # Changing email to one that exists should fail, regardless of case
     user.new_email = users(:public_user).email.upcase
@@ -517,7 +517,7 @@ class UserControllerTest < ActionController::TestCase
     assert_template :account
     assert_select ".notice", false
     assert_select "div#errorExplanation"
-    assert_select "form#accountForm > fieldset > div.form-row > div.field_with_errors > input#user_new_email"
+    assert_select "form#accountForm > fieldset > div.form-row > input.field_with_errors#user_new_email"
 
     # Changing email to one that doesn't exist should work
     user.new_email = "new_tester@example.com"

diff --git a/test/integration/user_creation_test.rb b/test/integration/user_creation_test.rb
index 179f9eae169f077b60df02ee9b2a2f2cf18e893c..6530b7ccbb2ca52c8e3326f7d413874fdcf9e4bc 100644 (file)
--- a/test/integration/user_creation_test.rb
+++ b/test/integration/user_creation_test.rb
@@ -29,7 +29,7 @@ class UserCreationTest < ActionDispatch::IntegrationTest
       assert_response :success
       assert_template 'user/new'
       assert_equal response.headers['Content-Language'][0..1], localer.to_s[0..1] unless localer == :root
-      assert_select "form > fieldset > div.form-row > div.field_with_errors > input#user_email"
+      assert_select "form > fieldset > div.form-row > input.field_with_errors#user_email"
       assert_no_missing_translations
     end
   end
@@ -47,7 +47,7 @@ class UserCreationTest < ActionDispatch::IntegrationTest
       end
       assert_response :success
       assert_template 'user/new'
-      assert_select "form > fieldset > div.form-row > div.field_with_errors > input#user_display_name"
+      assert_select "form > fieldset > div.form-row > input.field_with_errors#user_display_name"
       assert_no_missing_translations
     end
   end