Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle situation when I18n date.order values are strings #848

Merged
merged 1 commit into from
Jul 17, 2013
Merged

Handle situation when I18n date.order values are strings #848

merged 1 commit into from
Jul 17, 2013

Conversation

mjankowski
Copy link
Contributor

Rails 4 changes the date.order I18n setting to return strings, which
resulted in a missed lookup in POSITION, and a value like date_i
instead of date_1i for the for attribute on label elements.

position = ActionView::Helpers::DateTimeSelector::POSITION[position]
"#{attribute_name}_#{position}i"
position_value = ActionView::Helpers::DateTimeSelector::POSITION[position_key]
"#{attribute_name}_#{position_value}i"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any specific reason to change the variable names and not sticking with position only? It seems to just make the diff harder to follow since the real change is the to_sym call.

Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

position_key made more sense to me re: what the top bit is finding, so I went with it. Don't feel strongly.

Sent from my iPhone

On Jul 16, 2013, at 21:30, Carlos Antonio da Silva [email protected] wrote:

In lib/simple_form/inputs/date_time_input.rb:

  •    position = ActionView::Helpers::DateTimeSelector::POSITION[position]
    
  •    "#{attribute_name}_#{position}i"
    
  •    position_value = ActionView::Helpers::DateTimeSelector::POSITION[position_key]
    
  •    "#{attribute_name}_#{position_value}i"
    
    Any specific reason to change the variable names and not sticking with position only? It seems to just make the diff harder to follow since the real change is the to_sym call.

Thansk.


Reply to this email directly or view it on GitHub.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mjankowski we would prefer not to change this for now. Could you please revert it? :) Thanks!

Rails 4 changes the `date.order` I18n setting to return strings, which
resulted in a missed lookup in POSITION, and a value like `date_i`
instead of `date_1i` for the `for` attribute on `label` elements.
@mjankowski
Copy link
Contributor Author

Updated the commit to not change the position variable name.

nashby added a commit that referenced this pull request Jul 17, 2013
Handle situation when I18n date.order values are strings
@nashby nashby merged commit 87a9eec into heartcombo:master Jul 17, 2013
@nashby
Copy link
Collaborator

nashby commented Jul 17, 2013

@mjankowski thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants