From 81f2bdfccb6e72ac4b90e6ea24ce24c71e3074cb Mon Sep 17 00:00:00 2001 From: Jose Ivan Vargas Date: Fri, 10 Nov 2017 16:58:55 -0600 Subject: [PATCH 1/4] Removed tooltip from clone dropdown --- app/assets/javascripts/project.js | 31 +++++++++++++ app/helpers/button_helper.rb | 46 ++++++++----------- ...one-dropdown-should-not-have-a-tooltip.yml | 5 ++ 3 files changed, 56 insertions(+), 26 deletions(-) create mode 100644 changelogs/unreleased/39455-clone-dropdown-should-not-have-a-tooltip.yml diff --git a/app/assets/javascripts/project.js b/app/assets/javascripts/project.js index 36b6a5ed376..8d49ae2e712 100644 --- a/app/assets/javascripts/project.js +++ b/app/assets/javascripts/project.js @@ -14,6 +14,37 @@ export default class Project { $(`a:contains('${selectedCloneOption}')`, $cloneOptions).addClass('is-active'); } + $('a', $cloneOptions).on('click', (e) => { + const $this = $(e.currentTarget); + const url = $this.attr('href'); + const activeText = $this.find('.dropdown-menu-inner-title').text(); + + e.preventDefault(); + + $('.is-active', $cloneOptions).not($this).removeClass('is-active'); + $this.toggleClass('is-active'); + $projectCloneField.val(url); + $cloneBtnText.text(activeText); + + return $('.clone').text(url); + }); + // Ref switcher + this.initRefSwitcher(); + $('.project-refs-select').on('change', function() { + return $(this).parents('form').submit(); + }); + $('.hide-no-ssh-message').on('click', function(e) { + Cookies.set('hide_no_ssh_message', 'false'); + $(this).parents('.no-ssh-key-message').remove(); + return e.preventDefault(); + }); + $('.hide-no-password-message').on('click', function(e) { + Cookies.set('hide_no_password_message', 'false'); + $(this).parents('.no-password-message').remove(); + return e.preventDefault(); + }); + this.projectSelectDropdown(); + $('a', $cloneOptions).on('click', (e) => { const $this = $(e.currentTarget); const url = $this.attr('href'); diff --git a/app/helpers/button_helper.rb b/app/helpers/button_helper.rb index 8e8feeea1d8..fe5abf394a6 100644 --- a/app/helpers/button_helper.rb +++ b/app/helpers/button_helper.rb @@ -57,41 +57,35 @@ module ButtonHelper end def http_clone_button(project, placement = 'right', append_link: true) - klass = 'http-selector' - klass << ' has-tooltip' if current_user.try(:require_extra_setup_for_git_auth?) - protocol = gitlab_config.protocol.upcase - tooltip_title = - if current_user.try(:require_password_creation_for_git?) + protocol_description = + if current_user.try(:require_password_creation?) _("Set a password on your account to pull or push via %{protocol}.") % { protocol: protocol } else _("Create a personal access token on your account to pull or push via %{protocol}.") % { protocol: protocol } end - content_tag (append_link ? :a : :span), protocol, - class: klass, - href: (project.http_url_to_repo if append_link), - data: { - html: true, - placement: placement, - container: 'body', - title: tooltip_title - } + protocol_element_output = content_tag(:strong, protocol, class: 'dropdown-menu-inner-title') + + if current_user.try(:require_password_creation?) || current_user.try(:require_personal_access_token_creation_for_git_auth?) + protocol_element_output << content_tag(:span, protocol_description, class: 'dropdown-menu-inner-content') + end + + content_tag (append_link ? :a : :span), + protocol_element_output, + class: 'http-selector', + href: (project.http_url_to_repo if append_link) end - def ssh_clone_button(project, placement = 'right', append_link: true) - klass = 'ssh-selector' - klass << ' has-tooltip' if current_user.try(:require_ssh_key?) + def ssh_clone_button(project, append_link: true) + ssh_description = _('Add an SSH key to your profile to pull or push via SSH.') + ssh_element_output = content_tag(:strong, 'SSH', class: 'dropdown-menu-inner-title') + ssh_element_output << content_tag(:span, ssh_description, class: 'dropdown-menu-inner-content') if current_user.try(:require_ssh_key?) - content_tag (append_link ? :a : :span), 'SSH', - class: klass, - href: (project.ssh_url_to_repo if append_link), - data: { - html: true, - placement: placement, - container: 'body', - title: _('Add an SSH key to your profile to pull or push via SSH.') - } + content_tag (append_link ? :a : :span), + ssh_element_output, + class: 'ssh-selector', + href: (project.ssh_url_to_repo if append_link) end end diff --git a/changelogs/unreleased/39455-clone-dropdown-should-not-have-a-tooltip.yml b/changelogs/unreleased/39455-clone-dropdown-should-not-have-a-tooltip.yml new file mode 100644 index 00000000000..cb522cb7611 --- /dev/null +++ b/changelogs/unreleased/39455-clone-dropdown-should-not-have-a-tooltip.yml @@ -0,0 +1,5 @@ +--- +title: Removed tooltip from clone dropdown +merge_request: 15334 +author: +type: other From d19019ec926be2be066af7cae3ec8118be1062ab Mon Sep 17 00:00:00 2001 From: Jose Ivan Vargas Date: Fri, 10 Nov 2017 17:34:26 -0600 Subject: [PATCH 2/4] Change dropdown button background color --- app/assets/stylesheets/pages/projects.scss | 4 ++++ app/views/shared/_clone_panel.html.haml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index aaad6dbba8e..ce7d2e5c350 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -402,6 +402,10 @@ } } } + + .clone-dropdown-btn { + background-color: $white-light; + } } .project-repo-buttons { diff --git a/app/views/shared/_clone_panel.html.haml b/app/views/shared/_clone_panel.html.haml index 3d9c90c38fe..fba08092351 100644 --- a/app/views/shared/_clone_panel.html.haml +++ b/app/views/shared/_clone_panel.html.haml @@ -7,7 +7,7 @@ %span = enabled_project_button(project, enabled_protocol) - else - %a#clone-dropdown.clone-dropdown-btn.btn{ href: '#', data: { toggle: 'dropdown' } } + %a#clone-dropdown.btn.clone-dropdown-btn{ href: '#', data: { toggle: 'dropdown' } } %span = default_clone_protocol.upcase = icon('caret-down') From 2e2f0675c97ba233ab089922b231a7aac97d633a Mon Sep 17 00:00:00 2001 From: Jose Ivan Vargas Date: Mon, 13 Nov 2017 12:21:26 -0600 Subject: [PATCH 3/4] UX adjustments and spec corrections --- app/assets/javascripts/project.js | 30 ---------------------- app/assets/stylesheets/pages/projects.scss | 4 +++ app/helpers/application_settings_helper.rb | 4 +-- app/helpers/button_helper.rb | 4 +-- spec/helpers/button_helper_spec.rb | 12 +++++---- 5 files changed, 15 insertions(+), 39 deletions(-) diff --git a/app/assets/javascripts/project.js b/app/assets/javascripts/project.js index 8d49ae2e712..3131e71d9d6 100644 --- a/app/assets/javascripts/project.js +++ b/app/assets/javascripts/project.js @@ -29,36 +29,6 @@ export default class Project { return $('.clone').text(url); }); // Ref switcher - this.initRefSwitcher(); - $('.project-refs-select').on('change', function() { - return $(this).parents('form').submit(); - }); - $('.hide-no-ssh-message').on('click', function(e) { - Cookies.set('hide_no_ssh_message', 'false'); - $(this).parents('.no-ssh-key-message').remove(); - return e.preventDefault(); - }); - $('.hide-no-password-message').on('click', function(e) { - Cookies.set('hide_no_password_message', 'false'); - $(this).parents('.no-password-message').remove(); - return e.preventDefault(); - }); - this.projectSelectDropdown(); - - $('a', $cloneOptions).on('click', (e) => { - const $this = $(e.currentTarget); - const url = $this.attr('href'); - - e.preventDefault(); - - $('.is-active', $cloneOptions).not($this).removeClass('is-active'); - $this.toggleClass('is-active'); - $projectCloneField.val(url); - $cloneBtnText.text($this.text()); - - return $('.clone').text(url); - }); - // Ref switcher Project.initRefSwitcher(); $('.project-refs-select').on('change', function() { return $(this).parents('form').submit(); diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index ce7d2e5c350..26701a97ec9 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -406,6 +406,10 @@ .clone-dropdown-btn { background-color: $white-light; } + + .clone-options-dropdown { + min-width: 240px; + } } .project-repo-buttons { diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 6fc4248b245..77d77ad2fe1 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -30,9 +30,9 @@ module ApplicationSettingsHelper def enabled_project_button(project, protocol) case protocol when 'ssh' - ssh_clone_button(project, 'bottom', append_link: false) + ssh_clone_button(project, append_link: false) else - http_clone_button(project, 'bottom', append_link: false) + http_clone_button(project, append_link: false) end end diff --git a/app/helpers/button_helper.rb b/app/helpers/button_helper.rb index fe5abf394a6..4f945b5ad9e 100644 --- a/app/helpers/button_helper.rb +++ b/app/helpers/button_helper.rb @@ -56,7 +56,7 @@ module ButtonHelper end end - def http_clone_button(project, placement = 'right', append_link: true) + def http_clone_button(project, append_link: true) protocol = gitlab_config.protocol.upcase protocol_description = @@ -79,7 +79,7 @@ module ButtonHelper end def ssh_clone_button(project, append_link: true) - ssh_description = _('Add an SSH key to your profile to pull or push via SSH.') + ssh_description = _("You won't be able to pull or push project code via SSH until you add an SSH key to your profile") ssh_element_output = content_tag(:strong, 'SSH', class: 'dropdown-menu-inner-title') ssh_element_output << content_tag(:span, ssh_description, class: 'dropdown-menu-inner-content') if current_user.try(:require_ssh_key?) diff --git a/spec/helpers/button_helper_spec.rb b/spec/helpers/button_helper_spec.rb index e5158761333..b5222f301ba 100644 --- a/spec/helpers/button_helper_spec.rb +++ b/spec/helpers/button_helper_spec.rb @@ -26,9 +26,10 @@ describe ButtonHelper do context 'when user has password automatically set' do let(:user) { create(:user, password_automatically_set: true) } - it 'shows a password tooltip' do - expect(element.attr('class')).to include(has_tooltip_class) - expect(element.attr('data-title')).to eq('Set a password on your account to pull or push via HTTP.') + it 'shows the password text on the dropdown' do + expect(element.children.length).to eq(2) + expect(element.children[1].name).to eq('span') + expect(element.children[1].children[0].text).to eq('Set a password on your account to pull or push via HTTP.') end end end @@ -40,8 +41,9 @@ describe ButtonHelper do context 'when user has no personal access tokens' do it 'has a personal access token tooltip ' do - expect(element.attr('class')).to include(has_tooltip_class) - expect(element.attr('data-title')).to eq('Create a personal access token on your account to pull or push via HTTP.') + expect(element.children.length).to eq(2) + expect(element.children[1].name).to eq('span') + expect(element.children[1].children[0].text).to eq('Create a personal access token on your account to pull or push via HTTP.') end end From 349e3622f92cc4c6ea51ecaac76ef7c4ea8999e3 Mon Sep 17 00:00:00 2001 From: Jose Ivan Vargas Date: Wed, 15 Nov 2017 13:47:22 -0600 Subject: [PATCH 4/4] Added ssh_button helper specs and addressed ruby code observations --- app/assets/stylesheets/pages/projects.scss | 4 ++ app/helpers/button_helper.rb | 44 ++++++++--------- spec/helpers/button_helper_spec.rb | 55 ++++++++++++++++------ 3 files changed, 67 insertions(+), 36 deletions(-) diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 26701a97ec9..3799a3d4cc8 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -409,6 +409,10 @@ .clone-options-dropdown { min-width: 240px; + + .dropdown-menu-inner-content { + min-width: 320px; + } } } diff --git a/app/helpers/button_helper.rb b/app/helpers/button_helper.rb index 4f945b5ad9e..d06cf2de2c3 100644 --- a/app/helpers/button_helper.rb +++ b/app/helpers/button_helper.rb @@ -58,34 +58,34 @@ module ButtonHelper def http_clone_button(project, append_link: true) protocol = gitlab_config.protocol.upcase + dropdown_description = http_dropdown_description(protocol) + append_url = project.http_url_to_repo if append_link - protocol_description = - if current_user.try(:require_password_creation?) - _("Set a password on your account to pull or push via %{protocol}.") % { protocol: protocol } - else - _("Create a personal access token on your account to pull or push via %{protocol}.") % { protocol: protocol } - end + dropdown_item_with_description(protocol, dropdown_description, href: append_url) + end - protocol_element_output = content_tag(:strong, protocol, class: 'dropdown-menu-inner-title') - - if current_user.try(:require_password_creation?) || current_user.try(:require_personal_access_token_creation_for_git_auth?) - protocol_element_output << content_tag(:span, protocol_description, class: 'dropdown-menu-inner-content') + def http_dropdown_description(protocol) + if current_user.try(:require_password_creation_for_git?) + _("Set a password on your account to pull or push via %{protocol}.") % { protocol: protocol } + else + _("Create a personal access token on your account to pull or push via %{protocol}.") % { protocol: protocol } end - - content_tag (append_link ? :a : :span), - protocol_element_output, - class: 'http-selector', - href: (project.http_url_to_repo if append_link) end def ssh_clone_button(project, append_link: true) - ssh_description = _("You won't be able to pull or push project code via SSH until you add an SSH key to your profile") - ssh_element_output = content_tag(:strong, 'SSH', class: 'dropdown-menu-inner-title') - ssh_element_output << content_tag(:span, ssh_description, class: 'dropdown-menu-inner-content') if current_user.try(:require_ssh_key?) + dropdown_description = _("You won't be able to pull or push project code via SSH until you add an SSH key to your profile") if current_user.try(:require_ssh_key?) + append_url = project.ssh_url_to_repo if append_link - content_tag (append_link ? :a : :span), - ssh_element_output, - class: 'ssh-selector', - href: (project.ssh_url_to_repo if append_link) + dropdown_item_with_description('SSH', dropdown_description, href: append_url) + end + + def dropdown_item_with_description(title, description, href: nil) + button_content = content_tag(:strong, title, class: 'dropdown-menu-inner-title') + button_content << content_tag(:span, description, class: 'dropdown-menu-inner-content') if description + + content_tag (href ? :a : :span), + button_content, + class: "#{title.downcase}-selector", + href: (href if href) end end diff --git a/spec/helpers/button_helper_spec.rb b/spec/helpers/button_helper_spec.rb index b5222f301ba..d2c7867febb 100644 --- a/spec/helpers/button_helper_spec.rb +++ b/spec/helpers/button_helper_spec.rb @@ -27,9 +27,9 @@ describe ButtonHelper do let(:user) { create(:user, password_automatically_set: true) } it 'shows the password text on the dropdown' do - expect(element.children.length).to eq(2) - expect(element.children[1].name).to eq('span') - expect(element.children[1].children[0].text).to eq('Set a password on your account to pull or push via HTTP.') + description = element.search('.dropdown-menu-inner-content').first + + expect(description.inner_text).to eq 'Set a password on your account to pull or push via HTTP.' end end end @@ -40,18 +40,10 @@ describe ButtonHelper do end context 'when user has no personal access tokens' do - it 'has a personal access token tooltip ' do - expect(element.children.length).to eq(2) - expect(element.children[1].name).to eq('span') - expect(element.children[1].children[0].text).to eq('Create a personal access token on your account to pull or push via HTTP.') - end - end + it 'has a personal access token text on the dropdown description ' do + description = element.search('.dropdown-menu-inner-content').first - context 'when user has a personal access token' do - it 'shows no tooltip' do - create(:personal_access_token, user: user) - - expect(element.attr('class')).not_to include(has_tooltip_class) + expect(description.inner_text).to eq 'Create a personal access token on your account to pull or push via HTTP.' end end end @@ -65,6 +57,41 @@ describe ButtonHelper do end end + describe 'ssh_button' do + let(:user) { create(:user) } + let(:project) { build_stubbed(:project) } + + def element + element = helper.ssh_clone_button(project) + + Nokogiri::HTML::DocumentFragment.parse(element).first_element_child + end + + before do + allow(helper).to receive(:current_user).and_return(user) + end + + context 'without an ssh key on the user' do + it 'shows a warning on the dropdown description' do + description = element.search('.dropdown-menu-inner-content').first + + expect(description.inner_text).to eq "You won't be able to pull or push project code via SSH until you add an SSH key to your profile" + end + end + + context 'with an ssh key on the user' do + before do + create(:key, user: user) + end + + it 'there is no warning on the dropdown description' do + description = element.search('.dropdown-menu-inner-content').first + + expect(description).to eq nil + end + end + end + describe 'clipboard_button' do let(:user) { create(:user) } let(:project) { build_stubbed(:project) }