`date_select` helper method extension #13591

Open
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

kuldeepaggarwal commented Jan 4, 2014

This PR is inspired by #13552

<%= f.date_select :dob, { :start_year => 1900 } %>

When we attempt to create a resource, the year range is 1900 to (Date.today.year + 5). But after creation (with 1900 as dob), when resource is attempted to edit then the year range is rendered as 1900 to (selected year + 5). So if choose dob to:

  1. 1900 then in edit view, year range will be 1900-1905.
  2. 1920 then in edit view, year range will be 1900-1925.

Now with this PR, the end year will be: Date.today.year + 5, i.e. 2019 when dob is created either with 1905 or 1920. Same behaviour is implemented for start_year. And there is specially case for infinity.

@rafaelfranca rafaelfranca commented on the diff Jan 4, 2014

actionview/lib/action_view/helpers/date_helper.rb
else
- val = middle_year = year
+ val = year
+ if year.abs === Float::INFINITY
@rafaelfranca

rafaelfranca Jan 4, 2014

Owner

Why this infinity case?

@kuldeepaggarwal

kuldeepaggarwal Jan 4, 2014

Contributor

Checkout this test case, this is for infinity.

@rafaelfranca

rafaelfranca Jan 4, 2014

Owner

Ok. but why it is needed only now?

@rafaelfranca rafaelfranca commented on the diff Jan 4, 2014

actionview/lib/action_view/helpers/date_helper.rb
end
if @options[:use_hidden] || @options[:discard_year]
build_hidden(:year, val)
else
options = {}
- options[:start] = @options[:start_year] || middle_year - 5
- options[:end] = @options[:end_year] || middle_year + 5
+ options[:start] = @options[:start_year] || middle_years.min - 5
@rafaelfranca

rafaelfranca Jan 4, 2014

Owner

Why this min case? Lets take the simpler and safer path and change this code to take in consideration only the Data.today.year.

@kuldeepaggarwal

kuldeepaggarwal Jan 4, 2014

Contributor

yeah this is more safer 👍 I will change it.

@kuldeepaggarwal

kuldeepaggarwal Jan 4, 2014

Contributor

@rafaelfranca But suppose a user selected 1905 as selected date, then when we will try to edit the field we won't have the option in the DropDown list because DropDown menu will start from Date.today.year. So, i think we should go with this approach. wdyt?

@rafaelfranca

rafaelfranca Jan 4, 2014

Owner

How users will select 1905 if it is not disponible in the select box?

@kuldeepaggarwal

kuldeepaggarwal Jan 4, 2014

Contributor

e.g.
<%= f.date_select :dob %>, and say I choose my DOB to Date.today.year - 5, now I came after 1 year to edit my DOB, so now the DropDown list won't contain my previously selected option.

@rafaelfranca

rafaelfranca Jan 4, 2014

Owner

If we are going to support this so I think we should not change anything here. This will keep the surprising behavior where the actual selected value will change the start and end limits.

@rafaelfranca

rafaelfranca Jan 4, 2014

Owner

The only difference is that now the behavior is even more complicated to explain

@kuldeepaggarwal

kuldeepaggarwal Jan 4, 2014

Contributor

In the current system, the start and end limits are also dependent on the selected value. Am I wrong in this?

@rafaelfranca

rafaelfranca Jan 4, 2014

Owner

No, you are right. This is exactly what is causing confusion. If we are going to keep the implementation dependent on the selected value I prefer to keep it as it is since it is easier to explain than:

If there is a selected value the date_select helper will check if the selected value is lower than the current year and will calculate the start value subtracting 5 years.
@kuldeepaggarwal

kuldeepaggarwal Jan 4, 2014

Contributor

@rafaelfranca But there is an issue in that case, suppose we choose dob to Date.today.year - 5 and tried to save the object via form-submission and now because of some validation failure object does not save, now the user will have new list in the DropDown options. which wont have dates greater than Date.today.year.

@rafaelfranca

rafaelfranca Jan 4, 2014

Owner

Yes, I'm aware of this issue. What I'm trying to explain here is: we will not solve this problem adding more magic in this method. Adding more magic in the method will only add more confusion to users. After this "Why is the select taking in consideration the selected value?" is not a simple answer anymore.

For the issue you mentioned above I'd say, define the end value. If you think better if a validation error occurs hardly users will change the date.

I want to go simpler, or we change this behavior to do less magic or we keep it as it is. More magic is not an option here since it will only add more confusion.

Right now, I'm inclined to keep it as it is, since it will not require any change in the people application and the current behavior is documented.

@rafaelfranca

rafaelfranca Jan 4, 2014

Owner

I'll think better about this and do some tests with this branch.

Thank you for working on this ❤️

rafaelfranca was assigned Jan 4, 2014

Contributor

kuldeepaggarwal commented Jan 13, 2014

@rafaelfranca any updates on this?

@rafaelfranca rafaelfranca modified the milestone: 4.2.0, 5.0.0 Aug 18, 2014

Member

maclover7 commented Jan 10, 2016

Please rebase + address failing tests on Travis CI.

Contributor

kuldeepaggarwal commented Jan 10, 2016

Will do it tomorrow

@kuldeepaggarwal kuldeepaggarwal Previously `date_select` was rendering year range:
1. When no date is paased, `(Date.today.year - 5) to (Date.today.year + 5)`
2. When dats is passed, `(selected year minus 5) to (selected year plus 5)`

Now:

1. When no date is paased, `(Date.today.year - 5) to (Date.today.year + 5)`
2. When dats is passed, `([selected year, Date.today.year].min minus 5) to ([selected year, Date.today.year].max plus 5)` except in the case of infinity.
35204c7
Contributor

kuldeepaggarwal commented Jan 11, 2016

@maclover7 Updated the PR.

rafaelfranca removed this from the 5.0.0 [temp] milestone Apr 5, 2016

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