This project is archived and is in readonly mode.

#473 ✓wontfix
Cheah Chu Yeow

Don't pass ActiveRecordHelper label tags through field_error_proc

Reported by Cheah Chu Yeow | June 23rd, 2008 @ 03:55 PM

One of the 1st things I do with a new Rails project is to set a field_error_proc like so:

ActionView::Base.field_error_proc = Proc.new do |html_tag, instance|
  if html_tag =~ /<label/
    html_tag
  else
    # Handle every other tag.
  end
end

This trivial patch checks for a content_tag name argument of :label (because that is what Rails uses internally to generate a label tag) and skips error wrapping.

I don't really see a case for <label>s being wrapped around an error div.

Also, unrelated to this ticket, but I've also changed the #respond_to? calls to receive symbols rather than strings (it's cheaper).

Comments and changes to this ticket

  • josh

    josh September 30th, 2008 @ 05:55 PM

    • State changed from “new” to “open”
    • Milestone cleared.
    • Tag set to actionpack, patch, tiny

    Seems legit.

  • Rob Anderton

    Rob Anderton September 30th, 2008 @ 06:11 PM

    -1

    Not everybody uses field_error_proc to wrap fields in

    tags...I use it to append an 'error' class to elements, including labels.

    Sorry :)

  • Rob Anderton

    Rob Anderton September 30th, 2008 @ 06:12 PM

    Oops: that should read "in div tags"...

  • Cheah Chu Yeow

    Cheah Chu Yeow October 1st, 2008 @ 02:56 AM

    I know what you mean because my preferred field_error_proc does the same using Hpricot (i.e. inserting a CSS class instead of wrapping in needless divs).

    Your use of a CSS class on labels is definitely legit, so will gladly withdraw my patch unless someone else thinks it's valuable :) I still don't appreciate the default Rails' behavior though!

  • Rob Anderton

    Rob Anderton October 1st, 2008 @ 10:25 AM

    That's exactly what I do.

    I wonder if an alternative approach would be to allow something like this:

    
    ActionView::Base.on_field_error(:except => :label) do |html_tag, instance|
      # blah
    end
    

    So the on_field_error method takes an options hash that allows :only and :except options and a block which is called by the error wrapping code.

    Might be overkill for the sake of one 'if' though :D

  • Pratik

    Pratik October 9th, 2008 @ 08:28 PM

    • Milestone cleared.

    Too late for 2.2

  • Pratik

    Pratik January 11th, 2009 @ 03:17 PM

    • Milestone cleared.
    • Assigned user set to “josh”

    Not convinced this is a good idea, but Josh can decide that.

  • josh

    josh January 13th, 2009 @ 08:33 PM

    • Assigned user changed from “josh” to “Pratik”

    Handoff!

  • Pratik

    Pratik January 18th, 2009 @ 05:39 AM

    • State changed from “open” to “wontfix”

    Don't really think this is a good change. And now you can just use rails templates when generating a new app :)

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Attachments

Pages