From 9156882f8985e5e10af1d7122988a8af44be1f67 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 3 Mar 2015 13:22:43 -0800 Subject: [PATCH 1/6] Improve GitLab EE appearance * Fix bug with redirect to image logo after login * Use one (light) logo for header * Improve header logo preview * Dont require authentication for appearance images --- .../stylesheets/sections/appearances.scss | 9 ++++---- .../admin/appearances_controller.rb | 4 +--- app/controllers/uploads_controller.rb | 2 +- app/helpers/appearances_helper.rb | 21 +++++-------------- app/models/appearance.rb | 13 +----------- app/views/admin/appearances/_form.html.haml | 15 ++++--------- 6 files changed, 16 insertions(+), 48 deletions(-) diff --git a/app/assets/stylesheets/sections/appearances.scss b/app/assets/stylesheets/sections/appearances.scss index dd4a1b99ab..93c02f10f3 100644 --- a/app/assets/stylesheets/sections/appearances.scss +++ b/app/assets/stylesheets/sections/appearances.scss @@ -3,10 +3,9 @@ margin-bottom: 20px; } -.appearance-dark-logo-preview { - background-color: #F1F1F1; -} - .appearance-light-logo-preview { - background-color: #373737; + background-color: $style_color; + max-width: 72px; + padding: 10px; + margin-bottom: 10px; } diff --git a/app/controllers/admin/appearances_controller.rb b/app/controllers/admin/appearances_controller.rb index 51884ea6ec..6927740a43 100644 --- a/app/controllers/admin/appearances_controller.rb +++ b/app/controllers/admin/appearances_controller.rb @@ -37,11 +37,9 @@ class Admin::AppearancesController < Admin::ApplicationController def header_logos appearance = Appearance.last appearance.remove_light_logo! - appearance.remove_dark_logo! - appearance.save - redirect_to admin_appearances_path, notice: 'Header logos were succesfully removed.' + redirect_to admin_appearances_path, notice: 'Header logo were succesfully removed.' end private diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 810ac9f34b..906feb14ca 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -29,7 +29,7 @@ class UploadsController < ApplicationController private def authorize_access - unless params[:mounted_as] == 'avatar' + unless params[:mounted_as] == 'avatar' || params[:mounted_as] == 'light_logo' || params[:mounted_as] == 'logo' authenticate_user! && reject_blocked! end end diff --git a/app/helpers/appearances_helper.rb b/app/helpers/appearances_helper.rb index aa82b7ae65..4c7624bbb6 100644 --- a/app/helpers/appearances_helper.rb +++ b/app/helpers/appearances_helper.rb @@ -15,21 +15,6 @@ module AppearancesHelper end end - def brand_header_logo - if brand_item.header_logos? - haml_tag(:style) do - # Dark theme/light logo - haml_concat ".dark_theme .app_logo a h1 {" \ - "background: url('#{brand_item.light_logo}') " \ - "no-repeat center center !important; }" - # Light theme/dark logo - haml_concat ".light_theme .app_logo a h1 {" \ - "background: url('#{brand_item.dark_logo}') " \ - "no-repeat center center !important; }" - end - end - end - def brand_text markdown(brand_item.description) end @@ -39,6 +24,10 @@ module AppearancesHelper end def brand_header_logo - image_tag 'logo-white.png' + if brand_item.light_logo? + image_tag brand_item.light_logo + else + image_tag 'logo-white.png' + end end end diff --git a/app/models/appearance.rb b/app/models/appearance.rb index 2ac78199f8..1f8c4a2204 100644 --- a/app/models/appearance.rb +++ b/app/models/appearance.rb @@ -2,19 +2,8 @@ class Appearance < ActiveRecord::Base validates :title, presence: true validates :description, presence: true validates :logo, file_size: { maximum: 1000.kilobytes.to_i } - validates :dark_logo, - file_size: { maximum: 1000.kilobytes.to_i }, - presence: true, if: :light_logo? - - validates :light_logo, - file_size: { maximum: 1000.kilobytes.to_i }, - presence: true, if: :dark_logo? + validates :light_logo, file_size: { maximum: 1000.kilobytes.to_i } mount_uploader :logo, AttachmentUploader - mount_uploader :dark_logo, AttachmentUploader mount_uploader :light_logo, AttachmentUploader - - def header_logos? - dark_logo? && light_logo? - end end diff --git a/app/views/admin/appearances/_form.html.haml b/app/views/admin/appearances/_form.html.haml index 5370ba009e..cefd98784c 100644 --- a/app/views/admin/appearances/_form.html.haml +++ b/app/views/admin/appearances/_form.html.haml @@ -33,23 +33,16 @@ %legend Navigation bar: .form-group - = f.label :dark_logo, class: 'control-label' - .col-sm-10 - - if @appearance.dark_logo? - = image_tag @appearance.dark_logo, class: 'appearance-dark-logo-preview' - = f.file_field :dark_logo, class: "" - .hint - Maximum size is 1MB, page optimized for logo size 40x40px - = f.label :light_logo, class: 'control-label' + = f.label :light_logo, 'Header logo', class: 'control-label' .col-sm-10 - if @appearance.light_logo? = image_tag @appearance.light_logo, class: 'appearance-light-logo-preview' = f.file_field :light_logo, class: "" .hint - Maximum size is 1MB, page optimized for logo size 41x41px - -if @appearance.light_logo? || @appearance.dark_logo? + Maximum size is 1MB, page optimized for logo size 72x72px + -if @appearance.light_logo? %br - = link_to 'Remove header logos', header_logos_admin_appearances_path, data: { confirm: "Header logos will be removed. Are you sure?"}, method: :delete, class: "btn btn-remove btn-small remove-logo" + = link_to 'Remove header logo', header_logos_admin_appearances_path, data: { confirm: "Header logos will be removed. Are you sure?"}, method: :delete, class: "btn btn-remove btn-small remove-logo" .form-actions From 46d0f8f18324287dbf5df00b208b0166aa229517 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 3 Mar 2015 16:19:59 -0800 Subject: [PATCH 2/6] Fix tests --- app/helpers/appearances_helper.rb | 2 +- app/views/layouts/_head_panel.html.haml | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/helpers/appearances_helper.rb b/app/helpers/appearances_helper.rb index 4c7624bbb6..71eec15c2b 100644 --- a/app/helpers/appearances_helper.rb +++ b/app/helpers/appearances_helper.rb @@ -24,7 +24,7 @@ module AppearancesHelper end def brand_header_logo - if brand_item.light_logo? + if brand_item && brand_item.light_logo? image_tag brand_item.light_logo else image_tag 'logo-white.png' diff --git a/app/views/layouts/_head_panel.html.haml b/app/views/layouts/_head_panel.html.haml index 7e4ea03cbf..fc8a487ece 100644 --- a/app/views/layouts/_head_panel.html.haml +++ b/app/views/layouts/_head_panel.html.haml @@ -2,7 +2,6 @@ .navbar-inner .container %div.app_logo - - brand_header_logo if brand_item = link_to root_path, class: "home has_bottom_tooltip", title: "Dashboard" do = brand_header_logo %h1.title= title From bb57ee0260989be29213e8c514b56d4075ff0f0f Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 3 Mar 2015 16:21:52 -0800 Subject: [PATCH 3/6] Update ee changelog with header logo improvements --- CHANGELOG-EE | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG-EE b/CHANGELOG-EE index 9cc7585804..201c71376e 100644 --- a/CHANGELOG-EE +++ b/CHANGELOG-EE @@ -2,6 +2,7 @@ v 7.9.0 (unreleased) - Strip prefixes and suffixes from synced SSH keys: `SSHKey:ssh-rsa keykeykey` and `ssh-rsa keykeykey (SSH key)` will now work - Check if LDAP admin group exists before querying for user membership + - Use one custom header logo for all GitLab themes in appearance settings v 7.8.0 - Improved Jira issue closing integration From d56d4f25792eb6b226abb6fc4dc6b9254b007e86 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 3 Mar 2015 21:34:43 -0800 Subject: [PATCH 4/6] Fix custom header logo tests --- features/steps/admin/appearance.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/features/steps/admin/appearance.rb b/features/steps/admin/appearance.rb index 78b20e587a..4127d7af56 100644 --- a/features/steps/admin/appearance.rb +++ b/features/steps/admin/appearance.rb @@ -39,7 +39,6 @@ class Spinach::Features::AdminAppearance < Spinach::FeatureSteps step 'I attach header logos' do attach_file(:appearance_light_logo, File.join(Rails.root, 'public', 'header_logo_light.png')) - attach_file(:appearance_dark_logo, File.join(Rails.root, 'public', 'header_logo_dark.png')) click_button 'Save' end @@ -49,7 +48,6 @@ class Spinach::Features::AdminAppearance < Spinach::FeatureSteps step 'I should see header logos' do page.should have_xpath('//img[@src="/uploads/appearance/light_logo/1/header_logo_light.png"]') - page.should have_xpath('//img[@src="/uploads/appearance/dark_logo/1/header_logo_dark.png"]') end step 'I remove the logo' do @@ -57,7 +55,7 @@ class Spinach::Features::AdminAppearance < Spinach::FeatureSteps end step 'I remove the header logos' do - click_link 'Remove header logos' + click_link 'Remove header logo' end step 'I should see logo removed' do @@ -66,7 +64,6 @@ class Spinach::Features::AdminAppearance < Spinach::FeatureSteps step 'I should see header logos removed' do page.should_not have_xpath('//img[@src="/uploads/appearance/light_logo/1/header_logo_light.png"]') - page.should_not have_xpath('//img[@src="/uploads/appearance/dark_logo/1/header_logo_dark.png"]') end def appearance From e97153578ca75280ea2b94b0a02cec84a3c02b46 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 3 Mar 2015 22:15:38 -0800 Subject: [PATCH 5/6] Add appearance to allowed upload mounts --- app/controllers/uploads_controller.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 906feb14ca..5fb763aab9 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -29,7 +29,7 @@ class UploadsController < ApplicationController private def authorize_access - unless params[:mounted_as] == 'avatar' || params[:mounted_as] == 'light_logo' || params[:mounted_as] == 'logo' + unless %w(avatar logo light_logo).include?(params[:mounted_as]) authenticate_user! && reject_blocked! end end @@ -39,14 +39,15 @@ class UploadsController < ApplicationController user: User, project: Project, note: Note, - group: Group + group: Group, + appearance: Appearance } upload_models[params[:model].to_sym] end def upload_mount - upload_mounts = %w(avatar attachment file) + upload_mounts = %w(avatar attachment file logo light_logo) if upload_mounts.include?(params[:mounted_as]) params[:mounted_as] From eaf25df3185b4477717727fb616a1d1606c646c5 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 3 Mar 2015 22:22:18 -0800 Subject: [PATCH 6/6] Fix duplicating OAuth providers on login page --- app/views/devise/shared/_signin_box.html.haml | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/app/views/devise/shared/_signin_box.html.haml b/app/views/devise/shared/_signin_box.html.haml index 6c1610e25d..1fb65c3937 100644 --- a/app/views/devise/shared/_signin_box.html.haml +++ b/app/views/devise/shared/_signin_box.html.haml @@ -30,19 +30,3 @@ - elsif signin_enabled? = render 'devise/sessions/new_base' - - else - %div - No authentication methods configured. - -- if Gitlab.config.omniauth.enabled && devise_mapping.omniauthable? - .clearfix.prepend-top-20 - %p - %span.light - Sign in with   - - providers = additional_providers - - providers.each do |provider| - %span.light - - if default_providers.include?(provider) - = link_to authbutton(provider, 32), omniauth_authorize_path(resource_name, provider) - - else - = link_to provider.to_s.titleize, omniauth_authorize_path(resource_name, provider), class: "btn"