From 7479223d597a427831bc9d1ccde7cbc95ae922e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 11 Oct 2018 12:03:08 +0200 Subject: [PATCH] Wrap translation fields in a div This way we can show/hide that div when displaying translations, and we can remove the duplication applying the same logic to the label, the input, the error and the CKEditor. This way we also solve the problem of the textarea of the CKEditor taking space when we switch locales, as well as CKEditor itself taking space even when not displayed. --- app/helpers/translatable_form_helper.rb | 74 ++++++------------- .../legislation/draft_versions/_form.html.erb | 8 +- .../admin/legislation/questions_spec.rb | 11 +-- spec/shared/features/translatable.rb | 6 +- 4 files changed, 35 insertions(+), 64 deletions(-) diff --git a/app/helpers/translatable_form_helper.rb b/app/helpers/translatable_form_helper.rb index 14d746a4f..decab4dd7 100644 --- a/app/helpers/translatable_form_helper.rb +++ b/app/helpers/translatable_form_helper.rb @@ -10,14 +10,16 @@ module TranslatableFormHelper @object.globalize_locales.map do |locale| Globalize.with_locale(locale) do fields_for(:translations, translation_for(locale), builder: TranslationsFieldsBuilder) do |translations_form| - @template.concat translations_form.hidden_field( - :_destroy, - class: "destroy_locale", - data: { locale: locale }) + @template.content_tag :div, translations_options(translations_form.object, locale) do + @template.concat translations_form.hidden_field( + :_destroy, + class: "destroy_locale", + data: { locale: locale }) - @template.concat translations_form.hidden_field(:locale, value: locale) + @template.concat translations_form.hidden_field(:locale, value: locale) - yield translations_form + yield translations_form + end end end end.join.html_safe @@ -38,27 +40,24 @@ module TranslatableFormHelper translation.mark_for_destruction unless locale == I18n.locale end end + + private + + def translations_options(resource, locale) + { + class: " js-globalize-attribute", + style: @template.display_translation_style(resource.globalized_model, locale), + data: { locale: locale } + } + end end class TranslationsFieldsBuilder < FoundationRailsHelper::FormBuilder %i[text_field text_area cktext_area].each do |field| define_method field do |attribute, options = {}| - final_options = translations_options(options) - - label_help_text_and_field = - custom_label(attribute, final_options[:label], final_options[:label_options]) + - help_text(final_options[:hint]) + - super(attribute, final_options.merge(label: false, hint: false)) - - if field == :cktext_area - content_tag :div, - label_help_text_and_field, - class: "js-globalize-attribute", - style: display_style, - data: { locale: locale } - else - label_help_text_and_field - end + custom_label(attribute, options[:label], options[:label_options]) + + help_text(options[:hint]) + + super(attribute, options.merge(label: false, hint: false)) end end @@ -67,44 +66,17 @@ module TranslatableFormHelper end def label(attribute, text = nil, options = {}) - label_options = translations_options(options) + label_options = options.dup hint = label_options.delete(:hint) super(attribute, text, label_options) + help_text(hint) end - def display_style - @template.display_translation_style(@object.globalized_model, locale) - end - - def error_for(attribute, options = {}) - final_options = translations_options(options).merge(class: "error js-globalize-attribute") - - return unless has_error?(attribute) - - error_messages = object.errors[attribute].join(', ') - error_messages = error_messages.html_safe if options[:html_safe_errors] - content_tag(:small, error_messages, final_options) - end - private def help_text(text) if text - content_tag :span, text, - class: "help-text js-globalize-attribute", - data: { locale: locale }, - style: display_style - else - "" + content_tag :span, text, class: "help-text" end end - - def translations_options(options) - options.merge( - class: "#{options[:class]} js-globalize-attribute".strip, - style: "#{options[:style]} #{display_style}".strip, - data: (options[:data] || {}).merge(locale: locale) - ) - end end end diff --git a/app/views/admin/legislation/draft_versions/_form.html.erb b/app/views/admin/legislation/draft_versions/_form.html.erb index 417a303fe..93e955a0b 100644 --- a/app/views/admin/legislation/draft_versions/_form.html.erb +++ b/app/views/admin/legislation/draft_versions/_form.html.erb @@ -32,11 +32,7 @@ <%= translations_form.label :body, nil, hint: t("admin.legislation.draft_versions.form.use_markdown") %> - <%= content_tag :div, - class: "markdown-editor clear js-globalize-attribute", - data: { locale: translations_form.locale }, - style: translations_form.display_style do %> - +
<%= t("admin.legislation.draft_versions.form.title_html", @@ -66,7 +62,7 @@
- <% end %> +
<% end %>
diff --git a/spec/features/admin/legislation/questions_spec.rb b/spec/features/admin/legislation/questions_spec.rb index c4f390fa0..fa48b7e4f 100644 --- a/spec/features/admin/legislation/questions_spec.rb +++ b/spec/features/admin/legislation/questions_spec.rb @@ -127,12 +127,13 @@ feature 'Admin legislation questions' do edit_admin_legislation_process_question_path(question.process, question) end - let(:field_en) do - page.all("[data-locale='en'][id^='legislation_question_question_options'][id$='value']").first - end + let(:field_en) { field_for(:en) } + let(:field_es) { field_for(:es) } - let(:field_es) do - page.all("[data-locale='es'][id^='legislation_question_question_options'][id$='value']").first + def field_for(locale) + within(page.all(".translatable_fields[data-locale='#{locale}']").last) do + page.first("[id^='legislation_question_question_option'][id$='value']") + end end scenario 'Add translation for question option', :js do diff --git a/spec/shared/features/translatable.rb b/spec/shared/features/translatable.rb index f23b037a3..ba5f5f2d5 100644 --- a/spec/shared/features/translatable.rb +++ b/spec/shared/features/translatable.rb @@ -244,7 +244,9 @@ def field_for(field, locale, visible: true) if translatable_class.name == "I18nContent" "contents_content_#{translatable.key}values_#{field}_#{locale}" else - find("[data-locale='#{locale}'][id$='_#{field}']", visible: visible)[:id] + within(".translatable_fields[data-locale='#{locale}']") do + find("input[id$='_#{field}'], textarea[id$='_#{field}']", visible: visible)[:id] + end end end @@ -281,7 +283,7 @@ def expect_page_to_have_translatable_field(field, locale, with:) expect(page).to have_field field_for(field, locale), with: with click_link class: "fullscreen-toggle" elsif textarea_type == :ckeditor - within(".ckeditor div.js-globalize-attribute[data-locale='#{locale}']") do + within("div.js-globalize-attribute[data-locale='#{locale}'] .ckeditor ") do within_frame(0) { expect(page).to have_content with } end end