From 2bba743dc3111c5cfa8b920e68b8f5d297352bd2 Mon Sep 17 00:00:00 2001 From: Mantas Date: Mon, 27 Sep 2021 15:28:53 +0300 Subject: [PATCH] Fixes #2619 - KB header and footer link-color not changeable --- .../knowledge_base/public_menu_manager.coffee | 6 +- .../app/models/knowledge_base.coffee | 13 +++- .../app/views/knowledge_base/new_modal.coffee | 11 +-- .../public_menu_manager.jst.eco | 2 +- app/assets/stylesheets/zammad.scss | 2 +- app/models/knowledge_base.rb | 5 +- .../public/_inline_stylesheet.html.erb | 4 ++ ...0190531180304_initialize_knowledge_base.rb | 5 +- ...3172256_issue_2619_kb_header_link_color.rb | 11 +++ spec/factories/knowledge_base.rb | 1 + spec/models/knowledge_base_spec.rb | 8 +-- spec/support/custom_matchers.rb | 23 +++++++ .../admin/knowledge_base/public_menu_spec.rb | 34 ++++++++-- .../system/admin/knowledge_base/theme_spec.rb | 30 +++++++++ .../knowledge_base_public/menu_items_spec.rb | 67 +++++++++++++------ 15 files changed, 177 insertions(+), 45 deletions(-) create mode 100644 db/migrate/20210923172256_issue_2619_kb_header_link_color.rb create mode 100644 spec/system/admin/knowledge_base/theme_spec.rb diff --git a/app/assets/javascripts/app/controllers/knowledge_base/public_menu_manager.coffee b/app/assets/javascripts/app/controllers/knowledge_base/public_menu_manager.coffee index 879497e6d..34941450c 100644 --- a/app/assets/javascripts/app/controllers/knowledge_base/public_menu_manager.coffee +++ b/app/assets/javascripts/app/controllers/knowledge_base/public_menu_manager.coffee @@ -27,11 +27,13 @@ class App.KnowledgeBasePublicMenuManager extends App.Controller { headline: 'Header menu', identifier: 'header', - color: kb.color_header + color: kb.color_header, + color_link: kb.color_header_link }, { headline: 'Footer menu', - identifier: 'footer' + identifier: 'footer', + color_link: 'hsl(207,12%,50%)' } ] diff --git a/app/assets/javascripts/app/models/knowledge_base.coffee b/app/assets/javascripts/app/models/knowledge_base.coffee index a48f55bef..8bf326ba7 100644 --- a/app/assets/javascripts/app/models/knowledge_base.coffee +++ b/app/assets/javascripts/app/models/knowledge_base.coffee @@ -1,5 +1,5 @@ class App.KnowledgeBase extends App.Model - @configure 'KnowledgeBase', 'iconset', 'color_highlight', 'color_header', 'translation_ids', 'locale_ids', 'homepage_layout', 'category_layout', 'custom_address' + @configure 'KnowledgeBase', 'iconset', 'color_highlight', 'color_header', 'color_header_link', 'translation_ids', 'locale_ids', 'homepage_layout', 'category_layout', 'custom_address' @extend Spine.Model.Ajax @extend App.KnowledgeBaseActions @url: @apiPath + '/knowledge_bases' @@ -148,6 +148,17 @@ class App.KnowledgeBase extends App.Model display: false horizontal: true shown: true + }, { + name: 'color_header_link' + display: 'Header Link Color' + tag: 'color' + style: 'block' + null: false + screen: + admin_style_color_header_link: + display: false + horizontal: true + shown: true # Layout picker is disabled in V1 #}, { # name: 'homepage_layout' diff --git a/app/assets/javascripts/app/views/knowledge_base/new_modal.coffee b/app/assets/javascripts/app/views/knowledge_base/new_modal.coffee index 3dbb91fd8..bfd1905db 100644 --- a/app/assets/javascripts/app/views/knowledge_base/new_modal.coffee +++ b/app/assets/javascripts/app/views/knowledge_base/new_modal.coffee @@ -26,11 +26,12 @@ class App.KnowledgeBaseNewModal extends App.ControllerModal App.UiElement[attribute.tag].prepareParams?(attribute, dom, params) applyDefaults: (params) -> - params['iconset'] = 'FontAwesome' - params['color_highlight'] = '#38ae6a' - params['color_header'] = '#f9fafb' - params['homepage_layout'] = 'grid' - params['category_layout'] = 'grid' + params['iconset'] = 'FontAwesome' + params['color_highlight'] = '#38ae6a' + params['color_header'] = '#f9fafb' + params['color_header_link'] = 'hsl(206,8%,50%)' + params['homepage_layout'] = 'grid' + params['category_layout'] = 'grid' onSubmit: (e) -> params = @formParams(@el) diff --git a/app/assets/javascripts/app/views/knowledge_base/public_menu_manager.jst.eco b/app/assets/javascripts/app/views/knowledge_base/public_menu_manager.jst.eco index 3ccb6b454..7922cf902 100644 --- a/app/assets/javascripts/app/views/knowledge_base/public_menu_manager.jst.eco +++ b/app/assets/javascripts/app/views/knowledge_base/public_menu_manager.jst.eco @@ -14,7 +14,7 @@
<%= kb_locale.systemLocale().name %>
-
+
<% menu_items = App.KnowledgeBaseMenuItem.using_kb_locale_location(kb_locale, location.identifier) %> <% if menu_items.length == 0: %> diff --git a/app/assets/stylesheets/zammad.scss b/app/assets/stylesheets/zammad.scss index 5be9eecfd..62d8ff644 100644 --- a/app/assets/stylesheets/zammad.scss +++ b/app/assets/stylesheets/zammad.scss @@ -2661,7 +2661,7 @@ input.has-error { } a { - color: hsl(206,8%,50%); + color: inherit; } .label { diff --git a/app/models/knowledge_base.rb b/app/models/knowledge_base.rb index fac0bfec9..4a254e95b 100644 --- a/app/models/knowledge_base.rb +++ b/app/models/knowledge_base.rb @@ -27,8 +27,9 @@ class KnowledgeBase < ApplicationModel validates :category_layout, inclusion: { in: KnowledgeBase::LAYOUTS } validates :homepage_layout, inclusion: { in: KnowledgeBase::LAYOUTS } - validates :color_highlight, presence: true, color: true - validates :color_header, presence: true, color: true + validates :color_highlight, presence: true, color: true + validates :color_header, presence: true, color: true + validates :color_header_link, presence: true, color: true validates :iconset, inclusion: { in: KnowledgeBase::ICONSETS } diff --git a/app/views/knowledge_base/public/_inline_stylesheet.html.erb b/app/views/knowledge_base/public/_inline_stylesheet.html.erb index 0bcbfbeda..5cf8f590b 100644 --- a/app/views/knowledge_base/public/_inline_stylesheet.html.erb +++ b/app/views/knowledge_base/public/_inline_stylesheet.html.erb @@ -28,4 +28,8 @@ .header { background-color: <%= knowledge_base.color_header %>; } + + .header .menu-item { + color: <%= knowledge_base.color_header_link %>; + } diff --git a/db/migrate/20190531180304_initialize_knowledge_base.rb b/db/migrate/20190531180304_initialize_knowledge_base.rb index b4de691f5..d462e4374 100644 --- a/db/migrate/20190531180304_initialize_knowledge_base.rb +++ b/db/migrate/20190531180304_initialize_knowledge_base.rb @@ -8,8 +8,9 @@ class InitializeKnowledgeBase < ActiveRecord::Migration[5.0] create_table :knowledge_bases do |t| t.string :iconset, limit: 30, null: false - t.string :color_highlight, limit: 25, null: false - t.string :color_header, limit: 25, null: false + t.string :color_highlight, limit: 25, null: false + t.string :color_header, limit: 25, null: false + t.string :color_header_link, limit: 25, null: false t.string :homepage_layout, null: false t.string :category_layout, null: false diff --git a/db/migrate/20210923172256_issue_2619_kb_header_link_color.rb b/db/migrate/20210923172256_issue_2619_kb_header_link_color.rb new file mode 100644 index 000000000..8c7098004 --- /dev/null +++ b/db/migrate/20210923172256_issue_2619_kb_header_link_color.rb @@ -0,0 +1,11 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +class Issue2619KbHeaderLinkColor < ActiveRecord::Migration[6.0] + def up + return if !Setting.exists?(name: 'system_init_done') + + add_column :knowledge_bases, :color_header_link, :string, limit: 25, null: false, default: 'hsl(206,8%,50%)' + change_column_default :knowledge_bases, :color_header_link, nil + KnowledgeBasis.reset_column_information + end +end diff --git a/spec/factories/knowledge_base.rb b/spec/factories/knowledge_base.rb index 4ad0296a6..0e30049e2 100644 --- a/spec/factories/knowledge_base.rb +++ b/spec/factories/knowledge_base.rb @@ -8,6 +8,7 @@ FactoryBot.define do iconset { 'FontAwesome' } color_highlight { '#AAA' } color_header { '#EEE' } + color_header_link { '#FFF000' } homepage_layout { 'grid' } category_layout { 'list' } diff --git a/spec/models/knowledge_base_spec.rb b/spec/models/knowledge_base_spec.rb index 067d57ad1..65cfcbfa7 100644 --- a/spec/models/knowledge_base_spec.rb +++ b/spec/models/knowledge_base_spec.rb @@ -56,9 +56,9 @@ RSpec.describe KnowledgeBase, type: :model do let(:allowed_values) { ['#aaa', '#ff0000', 'rgb(0,100,100)', 'hsl(0,100%,50%)'] } let(:not_allowed_values) { ['aaa', '#aa', '#ff000', 'rgb(0,100,100', 'def(0,100%,0.5)', 'test'] } - it { is_expected.to allow_values(*allowed_values).for(:color_header) } - it { is_expected.to allow_values(*allowed_values).for(:color_highlight) } - it { is_expected.not_to allow_values(*not_allowed_values).for(:color_header) } - it { is_expected.not_to allow_values(*not_allowed_values).for(:color_highlight) } + %i[color_header color_header_link color_highlight].each do |attr| + it { is_expected.to allow_values(*allowed_values).for(attr) } + it { is_expected.not_to allow_values(*not_allowed_values).for(attr) } + end end end diff --git a/spec/support/custom_matchers.rb b/spec/support/custom_matchers.rb index d0b494ca3..ac8186455 100644 --- a/spec/support/custom_matchers.rb +++ b/spec/support/custom_matchers.rb @@ -7,3 +7,26 @@ RSpec::Matchers.define :exist_in_database do actual.class.exists?(actual.id) end end + +RSpec::Matchers.define :have_computed_style do + + match do + actual_value == expected_value + end + + failure_message do + "Expected element to have CSS property #{expected_key} with value #{expected_value}. But it was #{actual_value}." + end + + def expected_key + expected[0] + end + + def expected_value + expected[1] + end + + def actual_value + actual.evaluate_script "getComputedStyle(this).#{expected_key}" + end +end diff --git a/spec/system/admin/knowledge_base/public_menu_spec.rb b/spec/system/admin/knowledge_base/public_menu_spec.rb index e8829fb31..936518e62 100644 --- a/spec/system/admin/knowledge_base/public_menu_spec.rb +++ b/spec/system/admin/knowledge_base/public_menu_spec.rb @@ -7,20 +7,44 @@ RSpec.describe 'Admin Panel > Knowledge Base > Public Menu', type: :system do include_context 'basic Knowledge Base' include_context 'Knowledge Base menu items' - before do - visit '/#manage/knowledge_base' - find('a', text: 'Public Menu').click - end - context 'lists menu items' do + before do + visit '/#manage/knowledge_base' + find('a', text: 'Public Menu').click + end + it { expect(find_locale('Footer menu', alternative_locale).text).to include menu_item_4.title } it { expect(find_locale('Header menu', primary_locale).text).to include menu_item_1.title } it { expect(find_locale('Header menu', alternative_locale).text).not_to include menu_item_2.title } it { expect(find_locale('Header menu', primary_locale).text).to include menu_item_2.title } end + context 'menu items color' do + before do + knowledge_base.update! color_header_link: color + visit '/#manage/knowledge_base' + find('a', text: 'Public Menu').click + end + + let(:color) { 'rgb(255, 0, 255)' } + + it 'applies color for header preview' do + elem = all('.kb-menu-preview a')[0] + + expect(elem).to have_computed_style :color, color + end + + it 'does not apply color for footer preview' do + elem = all('.kb-menu-preview a')[3] + + expect(elem).not_to have_computed_style :color, color + end + end + context 'edit menu items' do before do + visit '/#manage/knowledge_base' + find('a', text: 'Public Menu').click find_location('Header menu').find('a', text: 'Edit').click modal_ready diff --git a/spec/system/admin/knowledge_base/theme_spec.rb b/spec/system/admin/knowledge_base/theme_spec.rb new file mode 100644 index 000000000..d73faa12b --- /dev/null +++ b/spec/system/admin/knowledge_base/theme_spec.rb @@ -0,0 +1,30 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +require 'rails_helper' + +# https://github.com/zammad/zammad/issues/266 +RSpec.describe 'Admin Panel > Knowledge Base > Theme', type: :system do + include_context 'basic Knowledge Base' + + context 'header link color' do + before do + knowledge_base + visit '/#manage/knowledge_base' + end + + it 'shows color' do + elem = find('#color_header_link input') + + expect(elem.value).to eq knowledge_base.color_header_link + end + + it 'saves new color' do + find('#color_header_link input').fill_in with: '#ccc' + find('#color_header_link button').click + + await_empty_ajax_queue + + expect(knowledge_base.reload.color_header_link).to eq '#ccc' + end + end +end diff --git a/spec/system/knowledge_base_public/menu_items_spec.rb b/spec/system/knowledge_base_public/menu_items_spec.rb index 53d803515..d36e8f320 100644 --- a/spec/system/knowledge_base_public/menu_items_spec.rb +++ b/spec/system/knowledge_base_public/menu_items_spec.rb @@ -6,35 +6,58 @@ RSpec.describe 'Public Knowledge Base menu items', type: :system, authenticated_ include_context 'basic Knowledge Base' include_context 'Knowledge Base menu items' - before do - published_answer + context 'menu items visibility' do + before do + published_answer - visit help_no_locale_path + visit help_no_locale_path + end + + it 'shows header public link' do + expect(page).to have_css('header .menu-item', text: menu_item_1.title) + end + + it 'shows another header public link' do + expect(page).to have_css('header .menu-item', text: menu_item_2.title) + end + + it "doesn't show footer link in header" do + expect(page).to have_no_css('header .menu-item', text: menu_item_3.title) + end + + it 'shows footer public link' do + expect(page).to have_css('footer .menu-item', text: menu_item_3.title) + end + + it "doesn't show footer link of another locale" do + expect(page).to have_no_css('footer .menu-item', text: menu_item_4.title) + end + + it 'shows public links in given order' do + index_1 = page.body.index menu_item_1.title + index_2 = page.body.index menu_item_2.title + expect(index_1).to be < index_2 + end end - it 'shows header public link' do - expect(page).to have_css('header .menu-item', text: menu_item_1.title) - end + context 'menu items color' do + before do + knowledge_base.update! color_header_link: color + visit help_no_locale_path + end - it 'shows another header public link' do - expect(page).to have_css('header .menu-item', text: menu_item_2.title) - end + let(:color) { 'rgb(255, 0, 255)' } - it "doesn't show footer link in header" do - expect(page).to have_no_css('header .menu-item', text: menu_item_3.title) - end + it 'applies color for header preview' do + elem = all('.menu-item')[0] - it 'shows footer public link' do - expect(page).to have_css('footer .menu-item', text: menu_item_3.title) - end + expect(elem).to have_computed_style :color, color + end - it "doesn't show footer link of another locale" do - expect(page).to have_no_css('footer .menu-item', text: menu_item_4.title) - end + it 'does not apply color for footer preview' do + elem = all('.menu-item')[2] - it 'shows public links in given order' do - index_1 = page.body.index menu_item_1.title - index_2 = page.body.index menu_item_2.title - expect(index_1).to be < index_2 + expect(elem).not_to have_computed_style :color, color + end end end