Rails 4 branch -- "for" attribute on form labels

[mjankowski]

Here's some output from a Rails 3.2 app with simple_form:

<label class="date required" for="user_date_of_birth_1i">
<abbr title="required">*</abbr> Date of birth</label>
<select class="date required" id="user_date_of_birth_1i" name="user[date_of_birth(1i)]">
<option value="2001">2001</option>

Here's the same output from the same app on a rails4 upgrade branch, using the simple_form rails4 branch:

<label class="date required" for="user_date_of_birth_i">
<abbr title="required">*</abbr> Date of birth</label>
<select class="date required" id="user_date_of_birth_1i" name="user[date_of_birth(1i)]">
<option value="2001">2001</option>
<option value="2000">2000</option>
<option value="1999">1999</option>

Note the difference there -- the for value of the surrounding label element has changed from user_date_of_birth_1i in the rails 3 app to user_date_of_birth_i in the rails4 app.

It's not clear to me if this:

• A bug in rails 4
• A bug in simple_form rails4 branch
• An intentional change in either Rails or SF which I need to do something about to fix in my app, set a config option, update a template, etc...

This change has the effect of making "click to focus" in browsers no longer work on these form elements, and of breaking tests which rely on capybara find_field, and the like.

[nashby]

That's interesting...We have specs that cover this behaviour https://github.com/plataformatec/simple_form/blob/master/test/inputs/datetime_input_test.rb#L65-L92

BTW, not sure about rails4 branch. Looks like we don't have it anymore. Could you please test it against master branch of SimpleForm?

[mjankowski]

Apologies, the described behavior was not with a rails4 branch, it's with master:

rapscallion:turpentine mjankowski$ bundle show simple_form
/Users/mjankowski/.rbenv/versions/2.0.0-p247/lib/ruby/gems/2.0.0/bundler/gems/simple_form-3ea3226e2f5e

[nashby]

@mjankowski could you please run this code in console?

I18n.t('date.order')

looks like you don't have this key.

[mjankowski]

Sure, I see:

[1] pry(main)> I18n.t('date.order')
=> ["year", "month", "day"]

[nashby]

I see. In rails 3.2 that code returns an array of symbols and in rails 4 it's an array of strings so we can't fetch the correct position here https://github.com/plataformatec/simple_form/blob/master/lib/simple_form/inputs/date_time_input.rb#L19
@rafaelfranca @carlosantoniodasilva any idea why this was changed?

[nashby]

rails/rails@6bb784e this. Well I think we should change SimpleForm's code too.

[carlosantoniodasilva]

👍 we can try fetching both symbol and string for now I guess.

[mjankowski]

I think that the hash keys in ActionView::Helpers::DateTimeSelector::POSITION are always symbols, it's just that the value of I18n.t('date.order') has changed...?

A solution might be to change line 19 in date_time_input to be:

    position = ActionView::Helpers::DateTimeSelector::POSITION[position.to_sym]

...but I'm not sure where to add test coverage. There are four tests in datetime_input_test which assume that date.order has symbols for keys. Do they all need alternate versions which use strings? Just one of them? Something new which only covers the string case?

[nashby]

Since SimpleForm 3.0 will support only Rails 4+ I think we should change all that tests to use strings. @carlosantoniodasilva right?

[mjankowski]

PR: #848

[carlosantoniodasilva]

@nashby seems fine yeah.

[carlosantoniodasilva]

(Just one thing to always keep in mind: users might still have the symbols in their locale files.)