From 4053a4046ebb032cc1938307fef4403def82a94d Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Thu, 16 Jun 2016 17:29:45 +0000 Subject: [PATCH] Merge branch 'template_dropdown' into 'master' Implements TemplateDropdown class to create custom template dropdowns ## What does this MR do? Refactorize template dropdowns. This MR creates a base TemplateSelector class so it can be reused for multiple types of templates. ## Does this MR meet the acceptance criteria? - [ ] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added - [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md) - [ ] API support added - [ ] Tests - [ ] Added for this feature/bug - [x] All builds are passing - [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if you do - rebase it please) - [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) See merge request !4697 --- .../blob/blob_gitignore_selector.js.coffee | 61 ++----------------- .../blob/blob_gitignore_selectors.js.coffee | 17 ++++++ .../blob/blob_license_selector.js.coffee | 35 +++-------- .../blob/blob_license_selectors.js.coffee | 17 ++++++ .../javascripts/blob/edit_blob.js.coffee | 5 +- .../blob/template_selector.js.coffee | 56 +++++++++++++++++ app/assets/stylesheets/pages/editor.scss | 3 +- app/helpers/blob_helper.rb | 4 +- app/views/projects/blob/_editor.html.haml | 10 ++- ...project_owner_creates_license_file_spec.rb | 14 ++++- ...eate_license_file_in_empty_project_spec.rb | 12 +++- .../labels/update_prioritization_spec.rb | 1 + 12 files changed, 133 insertions(+), 102 deletions(-) create mode 100644 app/assets/javascripts/blob/blob_gitignore_selectors.js.coffee create mode 100644 app/assets/javascripts/blob/blob_license_selectors.js.coffee create mode 100644 app/assets/javascripts/blob/template_selector.js.coffee diff --git a/app/assets/javascripts/blob/blob_gitignore_selector.js.coffee b/app/assets/javascripts/blob/blob_gitignore_selector.js.coffee index cc8a497d08..8d0e3f363d 100644 --- a/app/assets/javascripts/blob/blob_gitignore_selector.js.coffee +++ b/app/assets/javascripts/blob/blob_gitignore_selector.js.coffee @@ -1,58 +1,5 @@ -class @BlobGitignoreSelector - constructor: (opts) -> - { - @dropdown - @editor - @$wrapper = @dropdown.closest('.gitignore-selector') - @$filenameInput = $('#file_name') - @data = @dropdown.data('filenames') - } = opts +#= require blob/template_selector - @dropdown.glDropdown( - data: @data, - filterable: true, - selectable: true, - search: - fields: ['name'] - clicked: @onClick - text: (gitignore) -> - gitignore.name - ) - - @toggleGitignoreSelector() - @bindEvents() - - bindEvents: -> - @$filenameInput - .on 'keyup blur', (e) => - @toggleGitignoreSelector() - - toggleGitignoreSelector: -> - filename = @$filenameInput.val() or $('.editor-file-name').text().trim() - @$wrapper.toggleClass 'hidden', filename isnt '.gitignore' - - onClick: (item, el, e) => - e.preventDefault() - @requestIgnoreFile(item.name) - - requestIgnoreFile: (name) -> - Api.gitignoreText name, @requestIgnoreFileSuccess.bind(@) - - requestIgnoreFileSuccess: (gitignore) -> - @editor.setValue(gitignore.content, 1) - @editor.focus() - -class @BlobGitignoreSelectors - constructor: (opts) -> - { - @$dropdowns = $('.js-gitignore-selector') - @editor - } = opts - - @$dropdowns.each (i, dropdown) => - $dropdown = $(dropdown) - - new BlobGitignoreSelector( - dropdown: $dropdown, - editor: @editor - ) +class @BlobGitignoreSelector extends TemplateSelector + requestFile: (query) -> + Api.gitignoreText query.name, @requestFileSuccess.bind(@) diff --git a/app/assets/javascripts/blob/blob_gitignore_selectors.js.coffee b/app/assets/javascripts/blob/blob_gitignore_selectors.js.coffee new file mode 100644 index 0000000000..a719ba2512 --- /dev/null +++ b/app/assets/javascripts/blob/blob_gitignore_selectors.js.coffee @@ -0,0 +1,17 @@ +class @BlobGitignoreSelectors + constructor: (opts) -> + { + @$dropdowns = $('.js-gitignore-selector') + @editor + } = opts + + @$dropdowns.each (i, dropdown) => + $dropdown = $(dropdown) + + new BlobGitignoreSelector( + pattern: /(.gitignore)/, + data: $dropdown.data('data'), + wrapper: $dropdown.closest('.js-gitignore-selector-wrap'), + dropdown: $dropdown, + editor: @editor + ) diff --git a/app/assets/javascripts/blob/blob_license_selector.js.coffee b/app/assets/javascripts/blob/blob_license_selector.js.coffee index e17eaa75dc..a3cc8dd844 100644 --- a/app/assets/javascripts/blob/blob_license_selector.js.coffee +++ b/app/assets/javascripts/blob/blob_license_selector.js.coffee @@ -1,30 +1,9 @@ -class @BlobLicenseSelector - licenseRegex: /^(.+\/)?(licen[sc]e|copying)($|\.)/i +#= require blob/template_selector - constructor: (editor) -> - @$licenseSelector = $('.js-license-selector') - $fileNameInput = $('#file_name') +class @BlobLicenseSelector extends TemplateSelector + requestFile: (query) -> + data = + project: @dropdown.data('project') + fullname: @dropdown.data('fullname') - initialFileNameValue = if $fileNameInput.length - $fileNameInput.val() - else if $('.editor-file-name').length - $('.editor-file-name').text().trim() - - @toggleLicenseSelector(initialFileNameValue) - - if $fileNameInput - $fileNameInput.on 'keyup blur', (e) => - @toggleLicenseSelector($(e.target).val()) - - $('select.license-select').on 'change', (e) -> - data = - project: $(this).data('project') - fullname: $(this).data('fullname') - Api.licenseText $(this).val(), data, (license) -> - editor.setValue(license.content, -1) - - toggleLicenseSelector: (fileName) => - if @licenseRegex.test(fileName) - @$licenseSelector.show() - else - @$licenseSelector.hide() + Api.licenseText query.id, data, @requestFileSuccess.bind(@) diff --git a/app/assets/javascripts/blob/blob_license_selectors.js.coffee b/app/assets/javascripts/blob/blob_license_selectors.js.coffee new file mode 100644 index 0000000000..6843873310 --- /dev/null +++ b/app/assets/javascripts/blob/blob_license_selectors.js.coffee @@ -0,0 +1,17 @@ +class @BlobLicenseSelectors + constructor: (opts) -> + { + @$dropdowns = $('.js-license-selector') + @editor + } = opts + + @$dropdowns.each (i, dropdown) => + $dropdown = $(dropdown) + + new BlobLicenseSelector( + pattern: /^(.+\/)?(licen[sc]e|copying)($|\.)/i, + data: $dropdown.data('data'), + wrapper: $dropdown.closest('.js-license-selector-wrap'), + dropdown: $dropdown, + editor: @editor + ) diff --git a/app/assets/javascripts/blob/edit_blob.js.coffee b/app/assets/javascripts/blob/edit_blob.js.coffee index 79141e768b..636f909dbd 100644 --- a/app/assets/javascripts/blob/edit_blob.js.coffee +++ b/app/assets/javascripts/blob/edit_blob.js.coffee @@ -12,8 +12,9 @@ class @EditBlob $("#file-content").val(@editor.getValue()) @initModePanesAndLinks() - new BlobLicenseSelector(@editor) - new BlobGitignoreSelectors(editor: @editor) + + new BlobLicenseSelectors { @editor } + new BlobGitignoreSelectors { @editor } initModePanesAndLinks: -> @$editModePanes = $(".js-edit-mode-pane") diff --git a/app/assets/javascripts/blob/template_selector.js.coffee b/app/assets/javascripts/blob/template_selector.js.coffee new file mode 100644 index 0000000000..e76e303189 --- /dev/null +++ b/app/assets/javascripts/blob/template_selector.js.coffee @@ -0,0 +1,56 @@ +class @TemplateSelector + constructor: (opts = {}) -> + { + @dropdown, + @data, + @pattern, + @wrapper, + @editor, + @fileEndpoint, + @$input = $('#file_name') + } = opts + + @buildDropdown() + @bindEvents() + @onFilenameUpdate() + + buildDropdown: -> + @dropdown.glDropdown( + data: @data, + filterable: true, + selectable: true, + search: + fields: ['name'] + clicked: @onClick + text: (item) -> + item.name + ) + + bindEvents: -> + @$input.on('keyup blur', (e) => + @onFilenameUpdate() + ) + + onFilenameUpdate: -> + return unless @$input.length + + filenameMatches = @pattern.test(@$input.val().trim()) + + if not filenameMatches + @wrapper.addClass('hidden') + return + + @wrapper.removeClass('hidden') + + onClick: (item, el, e) => + e.preventDefault() + @requestFile(item) + + requestFile: (item) -> + # To be implemented on the extending class + # e.g. + # Api.gitignoreText item.name, @requestFileSuccess.bind(@) + + requestFileSuccess: (file) -> + @editor.setValue(file.content, 1) + @editor.focus() diff --git a/app/assets/stylesheets/pages/editor.scss b/app/assets/stylesheets/pages/editor.scss index 22679c764d..a34b06f105 100644 --- a/app/assets/stylesheets/pages/editor.scss +++ b/app/assets/stylesheets/pages/editor.scss @@ -66,8 +66,7 @@ font-family: $regular_font; } - .gitignore-selector { - + .gitignore-selector, .license-selector { .dropdown { line-height: 21px; } diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 85559fbc5f..5b54b34070 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -180,8 +180,8 @@ module BlobHelper licenses = Licensee::License.all @licenses_for_select = { - Popular: licenses.select(&:featured).map { |license| [license.name, license.key] }, - Other: licenses.reject(&:featured).map { |license| [license.name, license.key] } + Popular: licenses.select(&:featured).map { |license| { name: license.name, id: license.key } }, + Other: licenses.reject(&:featured).map { |license| { name: license.name, id: license.key } } } end diff --git a/app/views/projects/blob/_editor.html.haml b/app/views/projects/blob/_editor.html.haml index 4071b59c00..ae89637df6 100644 --- a/app/views/projects/blob/_editor.html.haml +++ b/app/views/projects/blob/_editor.html.haml @@ -13,12 +13,10 @@ required: true, class: 'form-control new-file-name' .pull-right - .license-selector.js-license-selector.hide - = select_tag :license_type, grouped_options_for_select(licenses_for_select, @project.repository.license_key), include_blank: true, class: 'select2 license-select', data: {placeholder: 'Choose a license template', project: @project.name, fullname: @project.namespace.human_name} - - .gitignore-selector.hidden - = dropdown_tag("Choose a .gitignore template", options: { toggle_class: 'js-gitignore-selector', title: "Choose a template", filter: true, placeholder: "Filter", data: { filenames: gitignore_names } } ) - + .license-selector.js-license-selector-wrap.hidden + = dropdown_tag("Choose a License template", options: { toggle_class: 'js-license-selector', title: "Choose a license", filter: true, placeholder: "Filter", data: { data: licenses_for_select, project: @project.name, fullname: @project.namespace.human_name } } ) + .gitignore-selector.js-gitignore-selector-wrap.hidden + = dropdown_tag("Choose a .gitignore template", options: { toggle_class: 'js-gitignore-selector', title: "Choose a template", filter: true, placeholder: "Filter", data: { data: gitignore_names } } ) .encoding-selector = select_tag :encoding, options_for_select([ "base64", "text" ], "text"), class: 'select2' diff --git a/spec/features/projects/files/project_owner_creates_license_file_spec.rb b/spec/features/projects/files/project_owner_creates_license_file_spec.rb index ecc818eb1e..e1e105e6bb 100644 --- a/spec/features/projects/files/project_owner_creates_license_file_spec.rb +++ b/spec/features/projects/files/project_owner_creates_license_file_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' feature 'project owner creates a license file', feature: true, js: true do - include Select2Helper + include WaitForAjax let(:project_master) { create(:user) } let(:project) { create(:project) } @@ -21,7 +21,7 @@ feature 'project owner creates a license file', feature: true, js: true do expect(page).to have_selector('.license-selector') - select2('mit', from: '#license_type') + select_template('MIT License') file_content = find('.file-content') expect(file_content).to have_content('The MIT License (MIT)') @@ -44,7 +44,7 @@ feature 'project owner creates a license file', feature: true, js: true do expect(find('#file_name').value).to eq('LICENSE') expect(page).to have_selector('.license-selector') - select2('mit', from: '#license_type') + select_template('MIT License') file_content = find('.file-content') expect(file_content).to have_content('The MIT License (MIT)') @@ -58,4 +58,12 @@ feature 'project owner creates a license file', feature: true, js: true do expect(page).to have_content('The MIT License (MIT)') expect(page).to have_content("Copyright (c) #{Time.now.year} #{project.namespace.human_name}") end + + def select_template(template) + page.within('.js-license-selector-wrap') do + click_button 'Choose a License template' + click_link template + wait_for_ajax + end + end end diff --git a/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb b/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb index 34eda29c28..67aac25e42 100644 --- a/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb +++ b/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' feature 'project owner sees a link to create a license file in empty project', feature: true, js: true do - include Select2Helper + include WaitForAjax let(:project_master) { create(:user) } let(:project) { create(:empty_project) } @@ -20,7 +20,7 @@ feature 'project owner sees a link to create a license file in empty project', f expect(find('#file_name').value).to eq('LICENSE') expect(page).to have_selector('.license-selector') - select2('mit', from: '#license_type') + select_template('MIT License') file_content = find('.file-content') expect(file_content).to have_content('The MIT License (MIT)') @@ -36,4 +36,12 @@ feature 'project owner sees a link to create a license file in empty project', f expect(page).to have_content('The MIT License (MIT)') expect(page).to have_content("Copyright (c) #{Time.now.year} #{project.namespace.human_name}") end + + def select_template(template) + page.within('.js-license-selector-wrap') do + click_button 'Choose a License template' + click_link template + wait_for_ajax + end + end end diff --git a/spec/features/projects/labels/update_prioritization_spec.rb b/spec/features/projects/labels/update_prioritization_spec.rb index 8550d279d0..6a39c302f5 100644 --- a/spec/features/projects/labels/update_prioritization_spec.rb +++ b/spec/features/projects/labels/update_prioritization_spec.rb @@ -77,6 +77,7 @@ feature 'Prioritize labels', feature: true do end visit current_url + wait_for_ajax page.within('.prioritized-labels') do expect(first('li')).to have_content('wontfix')