From fc9a87a8abf05ecaacb9134a4ac02470eb7363f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 27 Aug 2020 12:43:43 +0200 Subject: [PATCH] Use native HTML5 date fields in the admin section We've had to add a couple of hacks in order to make jQuery UI datepicker work with Turbolinks, and one of our tests is failing because the datepicker changes its height when changing from a month with 5 weeks to a month with 6 weeks. We could add a workaround so the test still passes (jQuery UI doesn't provide a configuration option to always displays 6 weeks in the datepicker), but I think it's easier to just use the HTML5 native date input field, which also allows us to simplify the code a bit and IMHO it improves the user experience, particularly when using mobile phones. Since date fields are not supported in Safari and Internet Explorer, we're still using the jQuery UI datepicker on those browsers (and on any other browser not supporting date fields). Due to these changes, we're moving the tests checking datepicker's behaviour to the dashboard. I've choosing not to change the public pages because I'm not 100% sure everybody would like this change (some people prefer the datepicker because we can configure the way it looks). --- app/assets/javascripts/advanced_search.js | 12 ++++ app/assets/javascripts/legislation_admin.js | 2 +- app/helpers/legislation_helper.rb | 4 -- app/views/admin/banners/_form.html.erb | 6 +- app/views/admin/budget_phases/_form.html.erb | 10 +--- .../legislation/processes/_form.html.erb | 60 ++++--------------- app/views/admin/milestones/_form.html.erb | 4 +- app/views/admin/poll/polls/_form.html.erb | 8 +-- lib/consul_form_builder.rb | 2 +- spec/system/admin/banners_spec.rb | 48 +-------------- .../admin/legislation/processes_spec.rb | 8 +-- spec/system/dashboard/polls_spec.rb | 38 ++++++++++++ 12 files changed, 77 insertions(+), 125 deletions(-) diff --git a/app/assets/javascripts/advanced_search.js b/app/assets/javascripts/advanced_search.js index e6ed92fa9..3f069747e 100644 --- a/app/assets/javascripts/advanced_search.js +++ b/app/assets/javascripts/advanced_search.js @@ -24,9 +24,21 @@ maxDate: "+0d" }); $(".js-calendar-full").datepicker(); + + if (!App.AdvancedSearch.browser_supports_date_field()) { + $("input[type='date']").datepicker(); + } + $.datepicker.setDefaults($.datepicker.regional[locale]); $.datepicker.setDefaults({ dateFormat: "dd/mm/yy" }); }, + browser_supports_date_field: function() { + var datefield; + + datefield = document.createElement("input"); + datefield.setAttribute("type", "date"); + return datefield.type === "date"; + }, initialize: function() { App.AdvancedSearch.init_calendar(); if (App.AdvancedSearch.advanced_search_terms()) { diff --git a/app/assets/javascripts/legislation_admin.js b/app/assets/javascripts/legislation_admin.js index 45d8723f4..efc7bf4ff 100644 --- a/app/assets/javascripts/legislation_admin.js +++ b/app/assets/javascripts/legislation_admin.js @@ -7,7 +7,7 @@ var checkbox; checkbox = $(this); - checkbox.closest("fieldset").find("input[type='text']").each(function() { + checkbox.closest("fieldset").find("input[type='date']").each(function() { if (checkbox.is(":checked")) { $(this).removeAttr("disabled"); } else { diff --git a/app/helpers/legislation_helper.rb b/app/helpers/legislation_helper.rb index 2aab71f27..679c8cf7e 100644 --- a/app/helpers/legislation_helper.rb +++ b/app/helpers/legislation_helper.rb @@ -3,10 +3,6 @@ module LegislationHelper l(date, format: "%d %b %Y") if date end - def format_date_for_calendar_form(date) - l(date, format: "%d/%m/%Y") if date - end - def new_legislation_proposal_link_text(process) t("proposals.index.start_proposal") end diff --git a/app/views/admin/banners/_form.html.erb b/app/views/admin/banners/_form.html.erb index e6b607650..9d9022b7b 100644 --- a/app/views/admin/banners/_form.html.erb +++ b/app/views/admin/banners/_form.html.erb @@ -7,16 +7,14 @@
<% date_started_at = @banner.post_started_at.present? ? I18n.localize(@banner.post_started_at) : "" %>
- <%= f.text_field :post_started_at, + <%= f.date_field :post_started_at, value: date_started_at, - class: "js-calendar-full", id: "post_started_at" %>
<% date_ended_at = @banner.post_ended_at.present? ? I18n.localize(@banner.post_ended_at) : "" %>
- <%= f.text_field :post_ended_at, + <%= f.date_field :post_ended_at, value: date_ended_at, - class: "js-calendar-full", id: "post_ended_at" %>
diff --git a/app/views/admin/budget_phases/_form.html.erb b/app/views/admin/budget_phases/_form.html.erb index f8e5cd964..609261344 100644 --- a/app/views/admin/budget_phases/_form.html.erb +++ b/app/views/admin/budget_phases/_form.html.erb @@ -6,16 +6,10 @@
- <%= f.text_field :starts_at, - value: format_date_for_calendar_form(@phase.starts_at), - class: "js-calendar-full", - id: "start_date" %> + <%= f.date_field :starts_at, id: "start_date" %>
- <%= f.text_field :ends_at, - value: format_date_for_calendar_form(@phase.ends_at), - class: "js-calendar-full", - id: "end_date" %> + <%= f.date_field :ends_at, id: "end_date" %>
diff --git a/app/views/admin/legislation/processes/_form.html.erb b/app/views/admin/legislation/processes/_form.html.erb index 16bfb5daf..eddba469a 100644 --- a/app/views/admin/legislation/processes/_form.html.erb +++ b/app/views/admin/legislation/processes/_form.html.erb @@ -11,17 +11,11 @@
- <%= f.text_field :draft_start_date, - value: format_date_for_calendar_form(@process.draft_start_date), - class: "js-calendar-full", - id: "draft_start_date" %> + <%= f.date_field :draft_start_date, id: "draft_start_date" %>
- <%= f.text_field :draft_end_date, - value: format_date_for_calendar_form(@process.draft_end_date), - class: "js-calendar-full", - id: "draft_end_date" %> + <%= f.date_field :draft_end_date, id: "draft_end_date" %>
<%= f.check_box :draft_phase_enabled, checked: @process.draft_phase.enabled?, label: t("admin.legislation.processes.form.enabled") %> @@ -34,17 +28,11 @@
- <%= f.text_field :start_date, - value: format_date_for_calendar_form(@process.start_date), - class: "js-calendar-full", - id: "start_date" %> + <%= f.date_field :start_date, id: "start_date" %>
- <%= f.text_field :end_date, - value: format_date_for_calendar_form(@process.end_date), - class: "js-calendar-full", - id: "end_date" %> + <%= f.date_field :end_date, id: "end_date" %>
<%= f.check_box :published, checked: @process.published?, label: t("admin.legislation.processes.form.enabled") %> @@ -57,17 +45,11 @@
- <%= f.text_field :debate_start_date, - value: format_date_for_calendar_form(@process.debate_start_date), - class: "js-calendar-full", - id: "debate_start_date" %> + <%= f.date_field :debate_start_date, id: "debate_start_date" %>
- <%= f.text_field :debate_end_date, - value: format_date_for_calendar_form(@process.debate_end_date), - class: "js-calendar-full", - id: "debate_end_date" %> + <%= f.date_field :debate_end_date, id: "debate_end_date" %>
<%= f.check_box :debate_phase_enabled, checked: @process.debate_phase.enabled?, label: t("admin.legislation.processes.form.enabled") %> @@ -80,17 +62,11 @@
- <%= f.text_field :proposals_phase_start_date, - value: format_date_for_calendar_form(@process.proposals_phase_start_date), - class: "js-calendar-full", - id: "proposals_phase_start_date" %> + <%= f.date_field :proposals_phase_start_date, id: "proposals_phase_start_date" %>
- <%= f.text_field :proposals_phase_end_date, - value: format_date_for_calendar_form(@process.proposals_phase_end_date), - class: "js-calendar-full", - id: "proposals_phase_end_date" %> + <%= f.date_field :proposals_phase_end_date, id: "proposals_phase_end_date" %>
<%= f.check_box :proposals_phase_enabled, checked: @process.proposals_phase.enabled?, label: t("admin.legislation.processes.form.enabled") %> @@ -103,17 +79,11 @@
- <%= f.text_field :allegations_start_date, - value: format_date_for_calendar_form(@process.allegations_start_date), - class: "js-calendar-full", - id: "allegations_start_date" %> + <%= f.date_field :allegations_start_date, id: "allegations_start_date" %>
- <%= f.text_field :allegations_end_date, - value: format_date_for_calendar_form(@process.allegations_end_date), - class: "js-calendar-full", - id: "allegations_end_date" %> + <%= f.date_field :allegations_end_date, id: "allegations_end_date" %>
<%= f.check_box :allegations_phase_enabled, checked: @process.allegations_phase.enabled?, label: t("admin.legislation.processes.form.enabled") %> @@ -122,10 +92,7 @@
- <%= f.text_field :draft_publication_date, - value: format_date_for_calendar_form(@process.draft_publication_date), - class: "js-calendar-full", - id: "draft_publication_date" %> + <%= f.date_field :draft_publication_date, id: "draft_publication_date" %>
<%= f.check_box :draft_publication_enabled, checked: @process.draft_publication.enabled?, label: t("admin.legislation.processes.form.enabled") %> @@ -134,10 +101,7 @@
- <%= f.text_field :result_publication_date, - value: format_date_for_calendar_form(@process.result_publication_date), - class: "js-calendar-full", - id: "result_publication_date" %> + <%= f.date_field :result_publication_date, id: "result_publication_date" %>
<%= f.check_box :result_publication_enabled, checked: @process.result_publication.enabled?, label: t("admin.legislation.processes.form.enabled") %> diff --git a/app/views/admin/milestones/_form.html.erb b/app/views/admin/milestones/_form.html.erb index 2a3bd75d0..a10a09aab 100644 --- a/app/views/admin/milestones/_form.html.erb +++ b/app/views/admin/milestones/_form.html.erb @@ -25,9 +25,7 @@
- <%= f.text_field :publication_date, - value: @milestone.publication_date.present? ? l(@milestone.publication_date.to_date) : nil, - class: "js-calendar-full" %> + <%= f.date_field :publication_date %> <%= render "images/admin_image", imageable: @milestone, f: f %> diff --git a/app/views/admin/poll/polls/_form.html.erb b/app/views/admin/poll/polls/_form.html.erb index 75b28dc40..4011ab4ec 100644 --- a/app/views/admin/poll/polls/_form.html.erb +++ b/app/views/admin/poll/polls/_form.html.erb @@ -6,15 +6,11 @@
- <%= f.text_field :starts_at, - value: @poll.starts_at.present? ? l(@poll.starts_at.to_date) : nil, - class: "js-calendar-full" %> + <%= f.date_field :starts_at %>
- <%= f.text_field :ends_at, - value: @poll.ends_at.present? ? l(@poll.ends_at.to_date) : nil, - class: "js-calendar-full" %> + <%= f.date_field :ends_at %>
diff --git a/lib/consul_form_builder.rb b/lib/consul_form_builder.rb index 066257d8f..778f0e8eb 100644 --- a/lib/consul_form_builder.rb +++ b/lib/consul_form_builder.rb @@ -9,7 +9,7 @@ class ConsulFormBuilder < FoundationRailsHelper::FormBuilder select attribute, choices, options, html_options end - %i[text_field text_area number_field password_field email_field].each do |field| + %i[text_field text_area date_field number_field password_field email_field].each do |field| define_method field do |attribute, options = {}| label_with_hint(attribute, options.merge(label_options: label_options_for(options))) + super(attribute, options.merge( diff --git a/spec/system/admin/banners_spec.rb b/spec/system/admin/banners_spec.rb index 9cd5e643c..7bd3f0d88 100644 --- a/spec/system/admin/banners_spec.rb +++ b/spec/system/admin/banners_spec.rb @@ -114,14 +114,9 @@ describe "Admin banners magement" do fill_in "Title", with: "En Français" fill_in "Description", with: "Link en Français" - fill_in "Link", with: "https://www.url.com" - - last_week = Time.current - 1.week - next_week = Time.current + 1.week - - fill_in "Post started at", with: last_week.strftime("%d/%m/%Y") - fill_in "Post ended at", with: next_week.strftime("%d/%m/%Y") + fill_in "Post started at", with: Time.current - 1.week + fill_in "Post ended at", with: Time.current + 1.week click_button "Save changes" click_link "Edit banner" @@ -179,45 +174,6 @@ describe "Admin banners magement" do expect(page).not_to have_content "Wrong text" end - scenario "when change date field on edit banner page display expected format", :js do - banner = create(:banner) - visit edit_admin_banner_path(banner) - - fill_in "Post started at", with: "20/02/2002" - find_field("Post started at").click - within(".ui-datepicker") { click_link "22" } - - expect(page).to have_field "Post started at", with: "22/02/2002" - end - - scenario "when date picker is opened and click on browser history back datepicker is closed", :js do - banner = create(:banner) - visit admin_banners_path(banner) - click_link "Edit banner" - find_field("Post started at").click - - expect(page).to have_css "#ui-datepicker-div" - - go_back - - expect(page).to have_content("Banners") - expect(page).not_to have_css "#ui-datepicker-div" - end - - scenario "date picker works after using the browser back button", :js do - banner = create(:banner) - - visit edit_admin_banner_path(banner) - click_link "Manage banners" - - expect(page).to have_link "Edit banner" - - go_back - find_field("Post started at").click - - expect(page).to have_css "#ui-datepicker-div" - end - scenario "Delete a banner" do create(:banner, title: "Ugly banner", description: "Bad text", diff --git a/spec/system/admin/legislation/processes_spec.rb b/spec/system/admin/legislation/processes_spec.rb index 1397f9feb..e85fdc353 100644 --- a/spec/system/admin/legislation/processes_spec.rb +++ b/spec/system/admin/legislation/processes_spec.rb @@ -162,8 +162,8 @@ describe "Admin collaborative legislation" do fill_in "Summary", with: "Summary of the process" base_date = Date.current - fill_in "legislation_process[start_date]", with: base_date.strftime("%d/%m/%Y") - fill_in "legislation_process[end_date]", with: (base_date + 5.days).strftime("%d/%m/%Y") + fill_in "legislation_process[start_date]", with: base_date + fill_in "legislation_process[end_date]", with: base_date + 5.days imageable_attach_new_file(create(:image), Rails.root.join("spec/fixtures/files/clippy.jpg")) click_button "Create process" @@ -258,8 +258,8 @@ describe "Admin collaborative legislation" do check "legislation_process[published]" - expect(page).to have_field "start_date", disabled: false, with: "07/07/2007" - expect(page).to have_field "end_date", disabled: false, with: "08/08/2008" + expect(page).to have_field "start_date", disabled: false, with: "2007-07-07" + expect(page).to have_field "end_date", disabled: false, with: "2008-08-08" end scenario "Enabling/disabling a phase does not enable/disable another phase date fields", :js do diff --git a/spec/system/dashboard/polls_spec.rb b/spec/system/dashboard/polls_spec.rb index 024624d17..9de22d6b6 100644 --- a/spec/system/dashboard/polls_spec.rb +++ b/spec/system/dashboard/polls_spec.rb @@ -41,6 +41,44 @@ describe "Polls" do expect(page).to have_content I18n.l(start_date.to_date) end + describe "Datepicker", :js do + scenario "displays the expected format when changing the date field" do + visit new_proposal_dashboard_poll_path(proposal) + + fill_in "Start Date", with: "20/02/2002" + find_field("Start Date").click + within(".ui-datepicker") { click_link "22" } + + expect(page).to have_field "Start Date", with: "22/02/2002" + end + + scenario "is closed after using the browser back button" do + visit proposal_dashboard_polls_path(proposal) + + click_link "Create poll" + find_field("Start Date").click + + expect(page).to have_css "#ui-datepicker-div" + + go_back + + expect(page).to have_link "Create poll" + expect(page).not_to have_css "#ui-datepicker-div" + end + + scenario "works after using the browser back button" do + visit new_proposal_dashboard_poll_path(proposal) + click_link "Polls" + + expect(page).to have_link "Create poll" + + go_back + find_field("Start Date").click + + expect(page).to have_css "#ui-datepicker-div" + end + end + scenario "Create a poll redirects back to form when invalid data", js: true do click_link "Polls" click_link "Create poll"