From 840d7ffd0bfde73584ff54dfee1c1c5288b325c1 Mon Sep 17 00:00:00 2001 From: Sascha Karnatz <122262394+sascha-karnatz@users.noreply.github.com> Date: Tue, 30 Apr 2024 15:11:37 +0200 Subject: [PATCH 1/3] Show fixed element tab group if the page has fixed elements Remove the step to create the tab group and always show the tab group, if the page has elements, that are fixed. It makes the whole process easier and is in the UI more consistent. --- app/assets/javascripts/alchemy/alchemy.fixed_elements.js | 9 --------- app/views/alchemy/admin/elements/create.js.erb | 3 --- app/views/alchemy/admin/elements/index.html.erb | 2 +- 3 files changed, 1 insertion(+), 13 deletions(-) diff --git a/app/assets/javascripts/alchemy/alchemy.fixed_elements.js b/app/assets/javascripts/alchemy/alchemy.fixed_elements.js index b52db9eb86..b8538c4c56 100644 --- a/app/assets/javascripts/alchemy/alchemy.fixed_elements.js +++ b/app/assets/javascripts/alchemy/alchemy.fixed_elements.js @@ -4,15 +4,6 @@ Alchemy.FixedElements = { WRAPPER: '', TABS: '{{label}}', - // Builds fixed elements tabs - buildTabs: function (label) { - var $wrapper = $(this.WRAPPER), - $tabs = $(this.TABS.replace(/{{label}}/, label)) - - $("#main-content-elements").wrap($wrapper) - $("#fixed-elements").prepend($tabs) - }, - // Creates a fixed element tab. createTab: function (element_id, label) { var $fixed_elements = $("#fixed-elements") diff --git a/app/views/alchemy/admin/elements/create.js.erb b/app/views/alchemy/admin/elements/create.js.erb index 00813e861a..28b1222326 100644 --- a/app/views/alchemy/admin/elements/create.js.erb +++ b/app/views/alchemy/admin/elements/create.js.erb @@ -7,9 +7,6 @@ <%- end -%> <% if @element.fixed? %> - if ($('#fixed-elements').length == 0) { - Alchemy.FixedElements.buildTabs('<%= Alchemy.t(:main_content) %>'); - } Alchemy.FixedElements.createTab('<%= @element.id %>', '<%= @element.display_name %>'); $element_area = $('[name="fixed-element-<%= @element.id %>"]'); <% elsif @element.parent_element %> diff --git a/app/views/alchemy/admin/elements/index.html.erb b/app/views/alchemy/admin/elements/index.html.erb index f9d33c37e0..338e912bbe 100644 --- a/app/views/alchemy/admin/elements/index.html.erb +++ b/app/views/alchemy/admin/elements/index.html.erb @@ -32,7 +32,7 @@ - <% if @fixed_elements.any? %> + <% if @page.element_definitions.any? { |el| el["fixed"] } %> <%= Alchemy.t(:main_content) %> From 1e59d5030c4dfd70bd4098dec6075dc62ab4e31f Mon Sep 17 00:00:00 2001 From: Sascha Karnatz <122262394+sascha-karnatz@users.noreply.github.com> Date: Tue, 30 Apr 2024 16:15:47 +0200 Subject: [PATCH 2/3] Move fixed elements functions into alchemy_admin package Move createTab and removeTab functions into alchemy_admin - package. These two functions are still available in the Alchemy.FixedElements - namespace until we move all calls out of these js.erb - files. The functions are also migrated to vanilla Javascript. --- app/assets/javascripts/alchemy/admin.js | 1 - .../alchemy/alchemy.fixed_elements.js | 36 ------------------- app/javascript/alchemy_admin.js | 2 ++ .../element_editor/delete_element_button.js | 3 +- .../alchemy_admin/fixed_elements.js | 24 +++++++++++++ 5 files changed, 28 insertions(+), 38 deletions(-) delete mode 100644 app/assets/javascripts/alchemy/alchemy.fixed_elements.js create mode 100644 app/javascript/alchemy_admin/fixed_elements.js diff --git a/app/assets/javascripts/alchemy/admin.js b/app/assets/javascripts/alchemy/admin.js index 4344b7c90d..037d4c6044 100644 --- a/app/assets/javascripts/alchemy/admin.js +++ b/app/assets/javascripts/alchemy/admin.js @@ -5,5 +5,4 @@ //= require handlebars //= require alchemy/templates //= require alchemy/alchemy.dialog -//= require alchemy/alchemy.fixed_elements //= require alchemy/alchemy.image_overlay diff --git a/app/assets/javascripts/alchemy/alchemy.fixed_elements.js b/app/assets/javascripts/alchemy/alchemy.fixed_elements.js deleted file mode 100644 index b8538c4c56..0000000000 --- a/app/assets/javascripts/alchemy/alchemy.fixed_elements.js +++ /dev/null @@ -1,36 +0,0 @@ -window.Alchemy = Alchemy || {} - -Alchemy.FixedElements = { - WRAPPER: '', - TABS: '{{label}}', - - // Creates a fixed element tab. - createTab: function (element_id, label) { - var $fixed_elements = $("#fixed-elements") - var panel_name = "fixed-element-" + element_id - - var $tab = - '' + label + "" - $fixed_elements.append($tab) - - var $panel = $( - '' - ) - $fixed_elements.append($panel) - window.requestAnimationFrame(function () { - $fixed_elements.get(0).show(panel_name) - }) - }, - - removeTab: function (element_id) { - var $fixed_elements = $("#fixed-elements") - - $fixed_elements - .find('sl-tab[panel="fixed-element-' + element_id + '"]') - .remove() - $fixed_elements - .find('sl-tab-panel[name="fixed-element-' + element_id + '"]') - .remove() - $fixed_elements.get(0).show("main-content-elements") - } -} diff --git a/app/javascript/alchemy_admin.js b/app/javascript/alchemy_admin.js index f88abd08c1..593812c7a0 100644 --- a/app/javascript/alchemy_admin.js +++ b/app/javascript/alchemy_admin.js @@ -6,6 +6,7 @@ import Rails from "@rails/ujs" import GUI from "alchemy_admin/gui" import { translate } from "alchemy_admin/i18n" import Dirty from "alchemy_admin/dirty" +import * as FixedElements from "alchemy_admin/fixed_elements" import { growl } from "alchemy_admin/growler" import IngredientAnchorLink from "alchemy_admin/ingredient_anchor_link" import ImageLoader from "alchemy_admin/image_loader" @@ -39,6 +40,7 @@ Object.assign(Alchemy, { ...Dirty, GUI, t: translate, // Global utility method for translating a given string + FixedElements, growl, ImageLoader: ImageLoader.init, ImageCropper, diff --git a/app/javascript/alchemy_admin/components/element_editor/delete_element_button.js b/app/javascript/alchemy_admin/components/element_editor/delete_element_button.js index 48b9a624b2..343dd170f4 100644 --- a/app/javascript/alchemy_admin/components/element_editor/delete_element_button.js +++ b/app/javascript/alchemy_admin/components/element_editor/delete_element_button.js @@ -1,3 +1,4 @@ +import { removeTab } from "alchemy_admin/fixed_elements" import { growl } from "alchemy_admin/growler" import { reloadPreview } from "alchemy_admin/components/preview_window" import { confirmToDeleteDialog } from "alchemy_admin/confirm_dialog" @@ -20,7 +21,7 @@ export class DeleteElementButton extends HTMLElement { const elementEditor = this.closest("alchemy-element-editor") elementEditor.addEventListener("transitionend", () => { if (elementEditor.fixed) { - Alchemy.FixedElements.removeTab(elementEditor.elementId) + removeTab(elementEditor.elementId) } elementEditor.remove() }) diff --git a/app/javascript/alchemy_admin/fixed_elements.js b/app/javascript/alchemy_admin/fixed_elements.js new file mode 100644 index 0000000000..78b5a1a888 --- /dev/null +++ b/app/javascript/alchemy_admin/fixed_elements.js @@ -0,0 +1,24 @@ +// Creates a fixed element tab. +export function createTab(element_id, label) { + const fixed_elements = document.getElementById("fixed-elements") + const panel_name = `fixed-element-${element_id}` + + const tab = `${label}` + const panel = `` + + fixed_elements.innerHTML += tab + panel + + window.requestAnimationFrame(function () { + fixed_elements.show(panel_name) + }) +} + +export function removeTab(element_id) { + const fixed_elements = document.getElementById("fixed-elements") + const panel_name = `fixed-element-${element_id}` + + fixed_elements.querySelector(`sl-tab[panel="${panel_name}"]`).remove() + fixed_elements.querySelector(`sl-tab-panel[name="${panel_name}"]`).remove() + + fixed_elements.show("main-content-elements") +} From da519a0e354b289960927220bb1e2ca485b4b29b Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 30 Apr 2024 18:14:58 +0200 Subject: [PATCH 3/3] fix(available_element_definitions): Do not mutate element_definitions `Page#available_element_definitions` returns elements still available for that page. Taking unique and limited elements into account. `Page#element_definitions` always returns the collections of defined elements, not matter of they have been used or not. Due to referencing the same instance variable and in memory object `available_element_definitions` was mutating the `element_definitions` object. --- app/models/alchemy/page/page_elements.rb | 16 ++++++++-------- spec/models/alchemy/page_spec.rb | 12 ++++++++++++ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/app/models/alchemy/page/page_elements.rb b/app/models/alchemy/page/page_elements.rb index 849391b530..503272d29b 100644 --- a/app/models/alchemy/page/page_elements.rb +++ b/app/models/alchemy/page/page_elements.rb @@ -80,21 +80,21 @@ def duplicate_elements(elements, repository, page_version) # type: Richtext # def available_element_definitions(only_element_named = nil) - @_element_definitions ||= if only_element_named + @_available_element_definitions ||= if only_element_named definition = Element.definition_by_name(only_element_named) element_definitions_by_name(definition["nestable_elements"]) else - element_definitions + element_definitions.dup end - return [] if @_element_definitions.blank? + return [] if @_available_element_definitions.blank? existing_elements = draft_version.elements.not_nested @_existing_element_names = existing_elements.pluck(:name) delete_unique_element_definitions! delete_outnumbered_element_definitions! - @_element_definitions + @_available_element_definitions end # All names of elements that can actually be placed on current page. @@ -186,18 +186,18 @@ def generate_elements end end - # Deletes unique and already present definitions from @_element_definitions. + # Deletes unique and already present definitions from @_available_element_definitions. # def delete_unique_element_definitions! - @_element_definitions.delete_if do |element| + @_available_element_definitions.delete_if do |element| element["unique"] && @_existing_element_names.include?(element["name"]) end end - # Deletes limited and outnumbered definitions from @_element_definitions. + # Deletes limited and outnumbered definitions from @_available_element_definitions. # def delete_outnumbered_element_definitions! - @_element_definitions.delete_if do |element| + @_available_element_definitions.delete_if do |element| outnumbered = @_existing_element_names.select { |name| name == element["name"] } element["amount"] && outnumbered.count >= element["amount"].to_i end diff --git a/spec/models/alchemy/page_spec.rb b/spec/models/alchemy/page_spec.rb index fe2665e83f..30e50c6c3f 100644 --- a/spec/models/alchemy/page_spec.rb +++ b/spec/models/alchemy/page_spec.rb @@ -590,6 +590,12 @@ module Alchemy expect(subject.collect { |e| e["name"] }).to include("article") expect(subject.collect { |e| e["name"] }).not_to include("header") end + + it "does not mutate the element_definitions collection" do + expect(page.element_definitions.collect { |e| e["name"] }).to include("header") + subject + expect(page.element_definitions.collect { |e| e["name"] }).to include("header") + end end context "limited amount" do @@ -650,6 +656,12 @@ module Alchemy it "should be ignored if unique" do expect(subject.collect { |e| e["name"] }).not_to include("unique_headline") end + + it "does not mutate the element_definitions collection" do + expect(page.element_definitions.collect { |e| e["name"] }).to include("column_headline") + subject + expect(page.element_definitions.collect { |e| e["name"] }).to include("column_headline") + end end describe ".ransackable_scopes" do