diff options
author | Eric Eastwood <contact@ericeastwood.com> | 2017-04-10 15:51:24 -0500 |
---|---|---|
committer | Eric Eastwood <contact@ericeastwood.com> | 2017-04-20 13:34:18 -0500 |
commit | 36387ce1b4a687a41f450c9fcccc348e478ca296 (patch) | |
tree | f22ef9dcfaef98e2bd643d63ec5e06fa46d28cfd | |
parent | 60a6389a7223f14156aeeec9a3854f8ea88de1f0 (diff) | |
download | gitlab-ce-36387ce1b4a687a41f450c9fcccc348e478ca296.tar.gz |
Add Fork/Cancel confirmation to "Replace"/"Delete" buttons30637-replace-delete-buttons-get-fork-cancel-confirmation
Fix https://gitlab.com/gitlab-org/gitlab-ce/issues/30637
-rw-r--r-- | app/assets/javascripts/blob/blob_fork_suggestion.js | 58 | ||||
-rw-r--r-- | app/assets/javascripts/commons/polyfills.js | 1 | ||||
-rw-r--r-- | app/assets/javascripts/dispatcher.js | 12 | ||||
-rw-r--r-- | app/helpers/blob_helper.rb | 34 | ||||
-rw-r--r-- | app/views/projects/blob/_header.html.haml | 11 | ||||
-rw-r--r-- | features/project/source/browse_files.feature | 4 | ||||
-rw-r--r-- | features/steps/project/source/browse_files.rb | 1 | ||||
-rw-r--r-- | spec/javascripts/blob/blob_fork_suggestion_spec.js | 37 |
8 files changed, 129 insertions, 29 deletions
diff --git a/app/assets/javascripts/blob/blob_fork_suggestion.js b/app/assets/javascripts/blob/blob_fork_suggestion.js index aa9a4e1c99a..3baf81905fe 100644 --- a/app/assets/javascripts/blob/blob_fork_suggestion.js +++ b/app/assets/javascripts/blob/blob_fork_suggestion.js @@ -1,15 +1,63 @@ -function BlobForkSuggestion(openButton, cancelButton, suggestionSection) { - if (openButton) { - openButton.addEventListener('click', () => { +const defaults = { + // Buttons that will show the `suggestionSections` + // has `data-fork-path`, and `data-action` + openButtons: [], + // Update the href(from `openButton` -> `data-fork-path`) + // whenever a `openButton` is clicked + forkButtons: [], + // Buttons to hide the `suggestionSections` + cancelButtons: [], + // Section to show/hide + suggestionSections: [], + // Pieces of text that need updating depending on the action, `edit`, `replace`, `delete` + actionTextPieces: [], +}; + +class BlobForkSuggestion { + constructor(options) { + this.elementMap = Object.assign({}, defaults, options); + this.onClickWrapper = this.onClick.bind(this); + + document.addEventListener('click', this.onClickWrapper); + } + + showSuggestionSection(forkPath, action = 'edit') { + [].forEach.call(this.elementMap.suggestionSections, (suggestionSection) => { suggestionSection.classList.remove('hidden'); }); + + [].forEach.call(this.elementMap.forkButtons, (forkButton) => { + forkButton.setAttribute('href', forkPath); + }); + + [].forEach.call(this.elementMap.actionTextPieces, (actionTextPiece) => { + // eslint-disable-next-line no-param-reassign + actionTextPiece.textContent = action; + }); } - if (cancelButton) { - cancelButton.addEventListener('click', () => { + hideSuggestionSection() { + [].forEach.call(this.elementMap.suggestionSections, (suggestionSection) => { suggestionSection.classList.add('hidden'); }); } + + onClick(e) { + const el = e.target; + + if ([].includes.call(this.elementMap.openButtons, el)) { + const { forkPath, action } = el.dataset; + this.showSuggestionSection(forkPath, action); + } + + if ([].includes.call(this.elementMap.cancelButtons, el)) { + this.hideSuggestionSection(); + } + } + + destroy() { + document.removeEventListener('click', this.onClickWrapper); + } } export default BlobForkSuggestion; diff --git a/app/assets/javascripts/commons/polyfills.js b/app/assets/javascripts/commons/polyfills.js index 3253eebd9b5..cb054a2a197 100644 --- a/app/assets/javascripts/commons/polyfills.js +++ b/app/assets/javascripts/commons/polyfills.js @@ -1,6 +1,7 @@ // ECMAScript polyfills import 'core-js/fn/array/find'; import 'core-js/fn/array/from'; +import 'core-js/fn/array/includes'; import 'core-js/fn/object/assign'; import 'core-js/fn/promise'; import 'core-js/fn/string/code-point-at'; diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 02a7df9b2a0..20db2698ba8 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -91,11 +91,13 @@ const ShortcutsBlob = require('./shortcuts_blob'); fileBlobPermalinkUrl, }); - new BlobForkSuggestion( - document.querySelector('.js-edit-blob-link-fork-toggler'), - document.querySelector('.js-cancel-fork-suggestion'), - document.querySelector('.js-file-fork-suggestion-section'), - ); + new BlobForkSuggestion({ + openButtons: document.querySelectorAll('.js-edit-blob-link-fork-toggler'), + forkButtons: document.querySelectorAll('.js-fork-suggestion-button'), + cancelButtons: document.querySelectorAll('.js-cancel-fork-suggestion-button'), + suggestionSections: document.querySelectorAll('.js-file-fork-suggestion-section'), + actionTextPieces: document.querySelectorAll('.js-file-fork-suggestion-section-action'), + }); } switch (page) { diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 4b3ab03a69c..3736e1ffcbb 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -14,15 +14,6 @@ module BlobHelper options[:link_opts]) end - def fork_path(project = @project, ref = @ref, path = @path, options = {}) - continue_params = { - to: edit_path, - notice: edit_in_new_fork_notice, - notice_now: edit_in_new_fork_notice_now - } - namespace_project_forks_path(project.namespace, project, namespace_key: current_user.namespace.id, continue: continue_params) - end - def edit_blob_link(project = @project, ref = @ref, path = @path, options = {}) blob = options.delete(:blob) blob ||= project.repository.blob_at(ref, path) rescue nil @@ -37,7 +28,16 @@ module BlobHelper elsif !current_user || (current_user && can_modify_blob?(blob, project, ref)) link_to 'Edit', edit_path(project, ref, path, options), class: "#{common_classes} btn-sm" elsif current_user && can?(current_user, :fork_project, project) - button_tag 'Edit', class: "#{common_classes} js-edit-blob-link-fork-toggler" + continue_params = { + to: edit_path, + notice: edit_in_new_fork_notice, + notice_now: edit_in_new_fork_notice_now + } + fork_path = namespace_project_forks_path(project.namespace, project, namespace_key: current_user.namespace.id, continue: continue_params) + + button_tag 'Edit', + class: "#{common_classes} js-edit-blob-link-fork-toggler", + data: { action: 'edit', fork_path: fork_path } end end @@ -48,21 +48,25 @@ module BlobHelper return unless blob + common_classes = "btn btn-#{btn_class}" + if !on_top_of_branch?(project, ref) - button_tag label, class: "btn btn-#{btn_class} disabled has-tooltip", title: "You can only #{action} files when you are on a branch", data: { container: 'body' } + button_tag label, class: "#{common_classes} disabled has-tooltip", title: "You can only #{action} files when you are on a branch", data: { container: 'body' } elsif blob.lfs_pointer? - button_tag label, class: "btn btn-#{btn_class} disabled has-tooltip", title: "It is not possible to #{action} files that are stored in LFS using the web interface", data: { container: 'body' } + button_tag label, class: "#{common_classes} disabled has-tooltip", title: "It is not possible to #{action} files that are stored in LFS using the web interface", data: { container: 'body' } elsif can_modify_blob?(blob, project, ref) - button_tag label, class: "btn btn-#{btn_class}", 'data-target' => "#modal-#{modal_type}-blob", 'data-toggle' => 'modal' + button_tag label, class: "#{common_classes}", 'data-target' => "#modal-#{modal_type}-blob", 'data-toggle' => 'modal' elsif can?(current_user, :fork_project, project) continue_params = { - to: request.fullpath, + to: request.fullpath, notice: edit_in_new_fork_notice + " Try to #{action} this file again.", notice_now: edit_in_new_fork_notice_now } fork_path = namespace_project_forks_path(project.namespace, project, namespace_key: current_user.namespace.id, continue: continue_params) - link_to label, fork_path, class: "btn btn-#{btn_class}", method: :post + button_tag label, + class: "#{common_classes} js-edit-blob-link-fork-toggler", + data: { action: action, fork_path: fork_path } end end diff --git a/app/views/projects/blob/_header.html.haml b/app/views/projects/blob/_header.html.haml index 7a4a293548c..d46e4534497 100644 --- a/app/views/projects/blob/_header.html.haml +++ b/app/views/projects/blob/_header.html.haml @@ -38,10 +38,15 @@ - if current_user = replace_blob_link = delete_blob_link + - if current_user .js-file-fork-suggestion-section.file-fork-suggestion.hidden %span.file-fork-suggestion-note - You don't have permission to edit this file. Try forking this project to edit the file. - = link_to 'Fork', fork_path, method: :post, class: 'btn btn-grouped btn-inverted btn-new' - %button.js-cancel-fork-suggestion.btn.btn-grouped{ type: 'button' } + You're not allowed to + %span.js-file-fork-suggestion-section-action + edit + files in this project directly. Please fork this project, + make your changes there, and submit a merge request. + = link_to 'Fork', nil, method: :post, class: 'js-fork-suggestion-button btn btn-grouped btn-inverted btn-new' + %button.js-cancel-fork-suggestion-button.btn.btn-grouped{ type: 'button' } Cancel diff --git a/features/project/source/browse_files.feature b/features/project/source/browse_files.feature index 536c24b6882..d81bc9802bc 100644 --- a/features/project/source/browse_files.feature +++ b/features/project/source/browse_files.feature @@ -117,6 +117,8 @@ Feature: Project Source Browse Files And I click on ".gitignore" file in repo And I see the ".gitignore" And I click on "Replace" + Then I should see a Fork/Cancel combo + And I click button "Fork" Then I should see a notice about a new fork having been created When I click on "Replace" And I replace it with a text file @@ -265,6 +267,8 @@ Feature: Project Source Browse Files And I click on ".gitignore" file in repo And I see the ".gitignore" And I click on "Delete" + Then I should see a Fork/Cancel combo + And I click button "Fork" Then I should see a notice about a new fork having been created When I click on "Delete" And I fill the commit message diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb index d52fa10c337..f5e8f7a7c32 100644 --- a/features/steps/project/source/browse_files.rb +++ b/features/steps/project/source/browse_files.rb @@ -377,7 +377,6 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps step 'I should see a Fork/Cancel combo' do expect(page).to have_link 'Fork' expect(page).to have_button 'Cancel' - expect(page).to have_content 'You don\'t have permission to edit this file. Try forking this project to edit the file.' end step 'I should see a notice about a new fork having been created' do diff --git a/spec/javascripts/blob/blob_fork_suggestion_spec.js b/spec/javascripts/blob/blob_fork_suggestion_spec.js new file mode 100644 index 00000000000..d0d64d75957 --- /dev/null +++ b/spec/javascripts/blob/blob_fork_suggestion_spec.js @@ -0,0 +1,37 @@ +import BlobForkSuggestion from '~/blob/blob_fork_suggestion'; + +describe('BlobForkSuggestion', () => { + let blobForkSuggestion; + + const openButtons = [document.createElement('div')]; + const forkButtons = [document.createElement('a')]; + const cancelButtons = [document.createElement('div')]; + const suggestionSections = [document.createElement('div')]; + const actionTextPieces = [document.createElement('div')]; + + beforeEach(() => { + blobForkSuggestion = new BlobForkSuggestion({ + openButtons, + forkButtons, + cancelButtons, + suggestionSections, + actionTextPieces, + }); + }); + + afterEach(() => { + blobForkSuggestion.destroy(); + }); + + it('showSuggestionSection', () => { + blobForkSuggestion.showSuggestionSection('/foo', 'foo'); + expect(suggestionSections[0].classList.contains('hidden')).toEqual(false); + expect(forkButtons[0].getAttribute('href')).toEqual('/foo'); + expect(actionTextPieces[0].textContent).toEqual('foo'); + }); + + it('hideSuggestionSection', () => { + blobForkSuggestion.hideSuggestionSection(); + expect(suggestionSections[0].classList.contains('hidden')).toEqual(true); + }); +}); |