From 5db3869a50997e1b7c41d69a654a5047ca236c24 Mon Sep 17 00:00:00 2001 From: Neeraj Singh Date: Thu, 29 Apr 2010 00:15:01 -0400 Subject: [PATCH] datetime_select and select_datetime should be consistent as much as possible date_select and select_date should be consistent as much as possible time_select and select_time should be consistent as much as possible [#4432 state:resolved] --- actionpack/lib/action_view/helpers/date_helper.rb | 76 +++++++--------- actionpack/test/template/date_helper_test.rb | 101 ++++++++++++++++----- 2 files changed, 113 insertions(+), 64 deletions(-) diff --git a/actionpack/lib/action_view/helpers/date_helper.rb b/actionpack/lib/action_view/helpers/date_helper.rb index 42018ee..8e64dc7 100644 --- a/actionpack/lib/action_view/helpers/date_helper.rb +++ b/actionpack/lib/action_view/helpers/date_helper.rb @@ -589,56 +589,50 @@ module ActionView @options = options.dup @html_options = html_options.dup @datetime = datetime + @options[:datetime_separator] ||= ' — ' + @options[:time_separator] ||= ' : ' end def select_datetime - # TODO: Remove tag conditional - # Ideally we could just join select_date and select_date for the tag case + order = date_order.dup + order -= [:hour, :minute, :second] + @options[:discard_year] ||= true unless order.include?(:year) + @options[:discard_month] ||= true unless order.include?(:month) + @options[:discard_day] ||= true if @options[:discard_month] || !order.include?(:day) + @options[:discard_minute] ||= true if @options[:discard_hour] + @options[:discard_second] ||= true unless @options[:include_seconds] && !@options[:discard_minute] + + # If the day is hidden and the month is visible, the day should be set to the 1st so all month choices are + # valid (otherwise it could be 31 and february wouldn't be a valid date) + if @datetime && @options[:discard_day] && !@options[:discard_month] + @datetime = @datetime.change(:day => 1) + end + if @options[:tag] && @options[:ignore_date] select_time - elsif @options[:tag] - order = date_order.dup - order -= [:hour, :minute, :second] - - @options[:discard_year] ||= true unless order.include?(:year) - @options[:discard_month] ||= true unless order.include?(:month) - @options[:discard_day] ||= true if @options[:discard_month] || !order.include?(:day) - @options[:discard_minute] ||= true if @options[:discard_hour] - @options[:discard_second] ||= true unless @options[:include_seconds] && !@options[:discard_minute] - - # If the day is hidden and the month is visible, the day should be set to the 1st so all month choices are - # valid (otherwise it could be 31 and february wouldn't be a valid date) - if @datetime && @options[:discard_day] && !@options[:discard_month] - @datetime = @datetime.change(:day => 1) - end - + else [:day, :month, :year].each { |o| order.unshift(o) unless order.include?(o) } order += [:hour, :minute, :second] unless @options[:discard_hour] build_selects_from_types(order) - else - "#{select_date}#{@options[:datetime_separator]}#{select_time}".html_safe end end def select_date order = date_order.dup - # TODO: Remove tag conditional - if @options[:tag] - @options[:discard_hour] = true - @options[:discard_minute] = true - @options[:discard_second] = true + @options[:discard_hour] = true + @options[:discard_minute] = true + @options[:discard_second] = true - @options[:discard_year] ||= true unless order.include?(:year) - @options[:discard_month] ||= true unless order.include?(:month) - @options[:discard_day] ||= true if @options[:discard_month] || !order.include?(:day) + @options[:discard_year] ||= true unless order.include?(:year) + @options[:discard_month] ||= true unless order.include?(:month) + @options[:discard_day] ||= true if @options[:discard_month] || !order.include?(:day) - # If the day is hidden and the month is visible, the day should be set to the 1st so all month choices are - # valid (otherwise it could be 31 and february wouldn't be a valid date) - if @datetime && @options[:discard_day] && !@options[:discard_month] - @datetime = @datetime.change(:day => 1) - end + # If the day is hidden and the month is visible, the day should be set to the 1st so all month choices are + # valid (otherwise it could be 31 and february wouldn't be a valid date) + if @datetime && @options[:discard_day] && !@options[:discard_month] + @datetime = @datetime.change(:day => 1) end [:day, :month, :year].each { |o| order.unshift(o) unless order.include?(o) } @@ -649,15 +643,12 @@ module ActionView def select_time order = [] - # TODO: Remove tag conditional - if @options[:tag] - @options[:discard_month] = true - @options[:discard_year] = true - @options[:discard_day] = true - @options[:discard_second] ||= true unless @options[:include_seconds] + @options[:discard_month] = true + @options[:discard_year] = true + @options[:discard_day] = true + @options[:discard_second] ||= true unless @options[:include_seconds] - order += [:year, :month, :day] unless @options[:ignore_date] - end + order += [:year, :month, :day] unless @options[:ignore_date] order += [:hour, :minute] order << :second if @options[:include_seconds] @@ -938,9 +929,8 @@ module ActionView options[:prefix] ||= @object_name options[:index] = @auto_index if @auto_index && !options.has_key?(:index) options[:datetime_separator] ||= ' — ' - options[:time_separator] ||= ' : ' - DateTimeSelector.new(datetime, options.merge(:tag => true), html_options) + DateTimeSelector.new(datetime, options, html_options) end def default_datetime(options) diff --git a/actionpack/test/template/date_helper_test.rb b/actionpack/test/template/date_helper_test.rb index da2477b..053fcc4 100644 --- a/actionpack/test/template/date_helper_test.rb +++ b/actionpack/test/template/date_helper_test.rb @@ -669,19 +669,11 @@ class DateHelperTest < ActionView::TestCase end def test_select_date_with_incomplete_order - # NOTE: modified this test because of minimal API change - expected = %(\n" - - expected << %(\n" - - expected << %(\n" - + # Since the order is incomplete nothing will be shown + expected = %(\n) + expected << %(\n) + expected << %(\n) + assert_dom_equal expected, select_date(Time.mktime(2003, 8, 16), :start_year => 2003, :end_year => 2005, :prefix => "date[first]", :order => [:day]) end @@ -903,10 +895,14 @@ class DateHelperTest < ActionView::TestCase expected << %(\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n) expected << "\n" + expected << " — " + expected << %(\n" + expected << " : " + expected << %(\n" @@ -955,10 +951,14 @@ class DateHelperTest < ActionView::TestCase expected << %(\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n) expected << "\n" + expected << " — " + expected << %(\n" + expected << " : " + expected << %(\n" @@ -971,6 +971,7 @@ class DateHelperTest < ActionView::TestCase expected << %(\n\n\n) expected << "\n" + expected << %(\n" @@ -979,10 +980,14 @@ class DateHelperTest < ActionView::TestCase expected << %(\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n) expected << "\n" + expected << " — " + expected << %(\n" + expected << " : " + expected << %(\n" @@ -1039,10 +1044,14 @@ class DateHelperTest < ActionView::TestCase expected << %(\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n) expected << "\n" + expected << " — " + expected << %(\n" + expected << " : " + expected << %(\n" @@ -1065,10 +1074,14 @@ class DateHelperTest < ActionView::TestCase expected << %(\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n) expected << "\n" + expected << " — " + expected << %(\n" + expected << " : " + expected << %(\n" @@ -1078,10 +1091,16 @@ class DateHelperTest < ActionView::TestCase end def test_select_time - expected = %(\n) + expected << %(\n) + expected << %(\n) + + expected << %(\n" + expected << " : " + expected << %(\n" @@ -1091,7 +1110,10 @@ class DateHelperTest < ActionView::TestCase end def test_select_time_with_separator - expected = %(\n) + expected << %(\n) + expected << %(\n) + expected << %(\n" @@ -1106,14 +1128,22 @@ class DateHelperTest < ActionView::TestCase end def test_select_time_with_seconds - expected = %(\n) + expected << %(\n) + expected << %(\n) + + expected << %(\n" + expected << ' : ' + expected << %(\n" + expected << ' : ' + expected << %(\n" @@ -1122,7 +1152,11 @@ class DateHelperTest < ActionView::TestCase end def test_select_time_with_seconds_and_separator - expected = %(\n) + expected << %(\n) + expected << %(\n) + + expected << %(\n" @@ -1142,10 +1176,16 @@ class DateHelperTest < ActionView::TestCase end def test_select_time_with_html_options - expected = %(\n) + expected << %(\n) + expected << %(\n) + + expected << %(\n" + expected << " : " + expected << %(\n" @@ -1159,14 +1199,22 @@ class DateHelperTest < ActionView::TestCase end def test_select_time_with_default_prompt - expected = %(\n) + expected << %(\n) + expected << %(\n) + + expected << %(\n" + + expected << " : " expected << %(\n" + expected << " : " + expected << %(\n" @@ -1175,15 +1223,22 @@ class DateHelperTest < ActionView::TestCase end def test_select_time_with_custom_prompt - - expected = %(\n) + expected << %(\n) + expected << %(\n) + + expected << %(\n" + expected << " : " + expected << %(\n" + expected << " : " + expected << %(\n" @@ -2006,10 +2061,14 @@ class DateHelperTest < ActionView::TestCase expected << %(\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n) expected << "\n" + expected << " — " + expected << %(\n" + expected << " : " + expected << %(\n" -- 1.6.5.2