From a76e2cffbda0159c02cfac0e99dd3de08610d082 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Wed, 8 Jan 2020 11:23:10 +0100 Subject: [PATCH 1/4] Simplify the element footer show-or-not code The only question relevant for showing the element footer is if we have content editors or tagging enabled. The nested elements are irrelevant for this. --- app/helpers/alchemy/admin/elements_helper.rb | 8 +--- .../alchemy/admin/elements/_element.html.erb | 11 ++---- .../alchemy/admin/elements_helper_spec.rb | 39 +++++-------------- 3 files changed, 16 insertions(+), 42 deletions(-) diff --git a/app/helpers/alchemy/admin/elements_helper.rb b/app/helpers/alchemy/admin/elements_helper.rb index 0a440b587c..0028fbfbea 100644 --- a/app/helpers/alchemy/admin/elements_helper.rb +++ b/app/helpers/alchemy/admin/elements_helper.rb @@ -102,13 +102,9 @@ def element_editor_classes(element) end # Tells us, if we should show the element footer. - def show_element_footer?(element, with_nestable_elements = nil) + def show_element_footer?(element) return false if element.folded? - if with_nestable_elements - element.content_definitions.present? || element.taggable? - else - element.nestable_elements.empty? - end + element.content_definitions.present? || element.taggable? end end end diff --git a/app/views/alchemy/admin/elements/_element.html.erb b/app/views/alchemy/admin/elements/_element.html.erb index 8f13162b35..dc49d80fcc 100644 --- a/app/views/alchemy/admin/elements/_element.html.erb +++ b/app/views/alchemy/admin/elements/_element.html.erb @@ -44,10 +44,11 @@ <% end %> <% end %> + <% if show_element_footer?(element) %> + <%= render 'alchemy/admin/elements/element_footer', element: element %> + <% end %> + <% if element.nestable_elements.any? %> - <% if show_element_footer?(element, :with_nestable_elements) %> - <%= render 'alchemy/admin/elements/element_footer', element: element %> - <% end %>
<%= content_tag :div, class: "nested-elements", data: { @@ -82,8 +83,4 @@ <% end %>
<% end %> - - <% if show_element_footer?(element) %> - <%= render 'alchemy/admin/elements/element_footer', element: element %> - <% end %> <% end %> diff --git a/spec/helpers/alchemy/admin/elements_helper_spec.rb b/spec/helpers/alchemy/admin/elements_helper_spec.rb index 0a701d2032..a38dab4baa 100644 --- a/spec/helpers/alchemy/admin/elements_helper_spec.rb +++ b/spec/helpers/alchemy/admin/elements_helper_spec.rb @@ -143,9 +143,8 @@ module Alchemy end describe "#show_element_footer?" do - subject { show_element_footer?(element, nestable_elements) } + subject { show_element_footer?(element) } let(:element) { build_stubbed(:alchemy_element) } - let(:nestable_elements) { nil } context "for folded element" do before { allow(element).to receive(:folded?) { true } } @@ -155,39 +154,21 @@ module Alchemy context "for expanded element" do before { allow(element).to receive(:folded?) { false } } - context "with nestable_elements argument" do - let(:nestable_elements) { true } - - context "and element having contents defined" do - before { allow(element).to receive(:content_definitions) { [1] } } - it { is_expected.to eq(true) } - end - - context "and element having no contents defined" do - before { allow(element).to receive(:content_definitions) { [] } } - - context "and element beeing taggable" do - before { allow(element).to receive(:taggable?) { true } } - it { is_expected.to eq(true) } - end - - context "and element not beeing taggable" do - before { allow(element).to receive(:taggable?) { false } } - it { is_expected.to eq(false) } - end - end + context "and element having contents defined" do + before { allow(element).to receive(:content_definitions) { [1] } } + it { is_expected.to eq(true) } end - context "without nestable_elements argument" do - let(:nestable_elements) { nil } + context "and element having no contents defined" do + before { allow(element).to receive(:content_definitions) { [] } } - context "and element having no nestable elements defined" do - before { allow(element).to receive(:nestable_elements) { [] } } + context "and element beeing taggable" do + before { allow(element).to receive(:taggable?) { true } } it { is_expected.to eq(true) } end - context "and element having nestable elements defined" do - before { allow(element).to receive(:nestable_elements) { [1] } } + context "and element not beeing taggable" do + before { allow(element).to receive(:taggable?) { false } } it { is_expected.to eq(false) } end end From 95f953bea251fde0a087aff9cefc9860a071e7ad Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Wed, 8 Jan 2020 11:42:09 +0100 Subject: [PATCH 2/4] Simplify the nestable elements style --- app/assets/stylesheets/alchemy/elements.scss | 31 ++++++++++---------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/app/assets/stylesheets/alchemy/elements.scss b/app/assets/stylesheets/alchemy/elements.scss index 1cffe0dda6..5c0675423b 100644 --- a/app/assets/stylesheets/alchemy/elements.scss +++ b/app/assets/stylesheets/alchemy/elements.scss @@ -155,6 +155,22 @@ } } + &.expanded { + &.not-fixed { + .nestable-elements { + box-shadow: inset 0 4px 8px -2px darken($medium-gray, 15%); + background-color: $medium-gray; + padding: 8px 4px 4px; + + .add-nestable-element-button { + width: calc(50% - 8px); + margin: 4px; + text-align: center; + } + } + } + } + &.dragged { border-style: dotted; overflow: hidden; @@ -762,21 +778,6 @@ textarea.has_tinymce { top: -1px; } -.not-fixed .nestable-elements { - box-shadow: inset 0 4px 8px -2px darken($medium-gray, 15%); - background-color: $medium-gray; - - .expanded.element-editor>& { - padding: 8px 4px 4px; - } - - .add-nestable-element-button { - width: calc(50% - 8px); - margin: 4px; - text-align: center; - } -} - .is-fixed { &.with-contents { >.element-footer { From fe2a59b79271cd913fcb60bd4474abe9968e9071 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Wed, 8 Jan 2020 11:44:58 +0100 Subject: [PATCH 3/4] Rename show_element_footer helper This helper actually tells us if the element is editable and if we should show the form --- app/helpers/alchemy/admin/elements_helper.rb | 4 ++-- spec/helpers/alchemy/admin/elements_helper_spec.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/helpers/alchemy/admin/elements_helper.rb b/app/helpers/alchemy/admin/elements_helper.rb index 0028fbfbea..40fddcbaa9 100644 --- a/app/helpers/alchemy/admin/elements_helper.rb +++ b/app/helpers/alchemy/admin/elements_helper.rb @@ -101,8 +101,8 @@ def element_editor_classes(element) ].join(' ') end - # Tells us, if we should show the element footer. - def show_element_footer?(element) + # Tells us, if we should show the element footer and form inputs. + def element_editable?(element) return false if element.folded? element.content_definitions.present? || element.taggable? end diff --git a/spec/helpers/alchemy/admin/elements_helper_spec.rb b/spec/helpers/alchemy/admin/elements_helper_spec.rb index a38dab4baa..2f754aa2f8 100644 --- a/spec/helpers/alchemy/admin/elements_helper_spec.rb +++ b/spec/helpers/alchemy/admin/elements_helper_spec.rb @@ -142,8 +142,8 @@ module Alchemy end end - describe "#show_element_footer?" do - subject { show_element_footer?(element) } + describe "#element_editable?" do + subject { element_editable?(element) } let(:element) { build_stubbed(:alchemy_element) } context "for folded element" do From e97fbe9afc002dc981ff7819adeca959e8f82f36 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Wed, 8 Jan 2020 12:06:55 +0100 Subject: [PATCH 4/4] Only show the element content editor form if necessary If we do not have any contents to edit we do not need to render the form at all --- app/assets/stylesheets/alchemy/elements.scss | 21 ++----- .../alchemy/admin/elements/_element.html.erb | 60 ++++++++++--------- 2 files changed, 36 insertions(+), 45 deletions(-) diff --git a/app/assets/stylesheets/alchemy/elements.scss b/app/assets/stylesheets/alchemy/elements.scss index 5c0675423b..906168badc 100644 --- a/app/assets/stylesheets/alchemy/elements.scss +++ b/app/assets/stylesheets/alchemy/elements.scss @@ -181,15 +181,6 @@ } } - &.with-contents, - &.without-contents.not-nestable { - - .element-content { - padding: 2*$default-padding 2*$default-padding 0; - border-top: 1px solid $medium-gray; - } - } - &.compact { .element-toolbar { visibility: hidden; @@ -210,7 +201,6 @@ } .element-footer { - margin-top: 0; padding-top: 0; border-top: 0; @@ -239,7 +229,7 @@ } .element-content { - padding: 4px 8px; + margin: 4px 8px; } .button_with_label { @@ -269,8 +259,8 @@ } } - form { - margin: 0; + .element-content { + margin: 2*$default-padding; } .validation_notice { @@ -282,8 +272,7 @@ } .message { - width: 100%; - margin: 2*$default-margin 0; + margin: 2*$default-margin; } .foot_note { @@ -348,6 +337,7 @@ .element-toolbar { padding: $default-padding 0; height: $element-toolbar-height; + border-bottom: 1px solid $medium-gray; .element_tools { float: left; @@ -357,7 +347,6 @@ .element-footer { border-top: 1px solid $medium-gray; - margin: 8px 0 0 0; padding: 2*$default-padding; text-align: right; diff --git a/app/views/alchemy/admin/elements/_element.html.erb b/app/views/alchemy/admin/elements/_element.html.erb index dc49d80fcc..dfe351edf8 100644 --- a/app/views/alchemy/admin/elements/_element.html.erb +++ b/app/views/alchemy/admin/elements/_element.html.erb @@ -10,42 +10,44 @@ <% if element.expanded? || element.fixed? %> <%= render 'alchemy/admin/elements/element_toolbar', element: element %> - <%= form_for [alchemy, :admin, element], remote: true, - html: {id: "element_#{element.id}_form".html_safe, class: 'element-content'} do |f| %> + <% element.definition[:message].tap do |message| %> + <%= render_message(:info, sanitize(message)) if message %> + <% end %> -
+ <% element.definition[:warning].tap do |warning| %> + <%= render_message(:warning, sanitize(warning)) if warning %> + <% end %> -
- <% element.definition[:message].tap do |message| %> - <%= render_message(:info, sanitize(message)) if message %> - <% end %> - <% element.definition[:warning].tap do |warning| %> - <%= render_message(:warning, sanitize(warning)) if warning %> - <% end %> - <% if lookup_context.exists?("#{element.name}_editor", ["alchemy/elements"], true) %> - <%= render_editor(element) %> - <% else %> - <%= element_editor_for(element) do %> - <% element.contents.each do |content| %> - <%= render "alchemy/essences/#{content.essence_partial_name}_editor", { - content: content - } %> + <% if element_editable?(element) %> + <%= form_for [alchemy, :admin, element], remote: true, + html: {id: "element_#{element.id}_form".html_safe, class: 'element-content'} do |f| %> + +
+ +
+ <% if lookup_context.exists?("#{element.name}_editor", ["alchemy/elements"], true) %> + <%= render_editor(element) %> + <% else %> + <%= element_editor_for(element) do %> + <% element.contents.each do |content| %> + <%= render "alchemy/essences/#{content.essence_partial_name}_editor", { + content: content + } %> + <% end %> <% end %> <% end %> - <% end %> -
- - <% if element.taggable? %> -
- <%= f.label :tag_list %> - <%= render 'alchemy/admin/partials/autocomplete_tag_list', f: f %>
+ + <% if element.taggable? %> +
+ <%= f.label :tag_list %> + <%= render 'alchemy/admin/partials/autocomplete_tag_list', f: f %> +
+ <% end %> <% end %> - <% end %> - <% end %> - <% if show_element_footer?(element) %> - <%= render 'alchemy/admin/elements/element_footer', element: element %> + <%= render 'alchemy/admin/elements/element_footer', element: element %> + <% end %> <% end %> <% if element.nestable_elements.any? %>