From 06376254b0f1afbeb60724079e78db7e9ecba21f Mon Sep 17 00:00:00 2001 From: Winnie Hellmann Date: Thu, 26 Oct 2017 13:03:44 +0200 Subject: [PATCH 1/6] Make NamespaceSelect a module --- app/assets/javascripts/dispatcher.js | 5 +- app/assets/javascripts/namespace_select.js | 135 +++++++++------------ 2 files changed, 59 insertions(+), 81 deletions(-) diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index f20162c48e9..cda19d43717 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -16,7 +16,7 @@ import CILintEditor from './ci_lint_editor'; /* global GroupsSelect */ /* global Search */ /* global Admin */ -/* global NamespaceSelects */ +import NamespaceSelect from './namespace_select'; /* global NewCommitForm */ /* global NewBranchForm */ /* global Project */ @@ -571,7 +571,8 @@ import Diff from './diff'; new UsersSelect(); break; case 'projects': - new NamespaceSelects(); + document.querySelectorAll('.js-namespace-select') + .forEach(dropdown => new NamespaceSelect({ dropdown })); break; case 'labels': switch (path[2]) { diff --git a/app/assets/javascripts/namespace_select.js b/app/assets/javascripts/namespace_select.js index 5da2db063a4..de987b3d947 100644 --- a/app/assets/javascripts/namespace_select.js +++ b/app/assets/javascripts/namespace_select.js @@ -1,85 +1,62 @@ /* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, one-var, vars-on-top, one-var-declaration-per-line, comma-dangle, object-shorthand, no-else-return, prefer-template, quotes, prefer-arrow-callback, no-param-reassign, no-cond-assign, max-len */ import Api from './api'; -(function() { - window.NamespaceSelect = (function() { - function NamespaceSelect(opts) { - this.onSelectItem = this.onSelectItem.bind(this); - var fieldName, showAny; - this.dropdown = opts.dropdown; - showAny = true; - fieldName = 'namespace_id'; - if (this.dropdown.attr('data-field-name')) { - fieldName = this.dropdown.data('fieldName'); - } - if (this.dropdown.attr('data-show-any')) { - showAny = this.dropdown.data('showAny'); - } - this.dropdown.glDropdown({ - filterable: true, - selectable: true, - filterRemote: true, - search: { - fields: ['path'] - }, - fieldName: fieldName, - toggleLabel: function(selected) { - if (selected.id == null) { - return selected.text; - } else { - return selected.kind + ": " + selected.full_path; - } - }, - data: function(term, dataCallback) { - return Api.namespaces(term, function(namespaces) { - var anyNamespace; - if (showAny) { - anyNamespace = { - text: 'Any namespace', - id: null - }; - namespaces.unshift(anyNamespace); - namespaces.splice(1, 0, 'divider'); - } - return dataCallback(namespaces); - }); - }, - text: function(namespace) { - if (namespace.id == null) { - return namespace.text; - } else { - return namespace.kind + ": " + namespace.full_path; - } - }, - renderRow: this.renderRow, - clicked: this.onSelectItem - }); +export default class NamespaceSelect { + constructor(opts) { + this.onSelectItem = this.onSelectItem.bind(this); + var fieldName, showAny; + this.dropdown = $(opts.dropdown); + showAny = true; + fieldName = 'namespace_id'; + if (this.dropdown.attr('data-field-name')) { + fieldName = this.dropdown.data('fieldName'); } - - NamespaceSelect.prototype.onSelectItem = function(options) { - const { e } = options; - return e.preventDefault(); - }; - - return NamespaceSelect; - })(); - - window.NamespaceSelects = (function() { - function NamespaceSelects(opts) { - var ref; - if (opts == null) { - opts = {}; - } - this.$dropdowns = (ref = opts.$dropdowns) != null ? ref : $('.js-namespace-select'); - this.$dropdowns.each(function(i, dropdown) { - var $dropdown; - $dropdown = $(dropdown); - return new window.NamespaceSelect({ - dropdown: $dropdown + if (this.dropdown.attr('data-show-any')) { + showAny = this.dropdown.data('showAny'); + } + this.dropdown.glDropdown({ + filterable: true, + selectable: true, + filterRemote: true, + search: { + fields: ['path'] + }, + fieldName: fieldName, + toggleLabel: function(selected) { + if (selected.id == null) { + return selected.text; + } else { + return selected.kind + ": " + selected.full_path; + } + }, + data: function(term, dataCallback) { + return Api.namespaces(term, function(namespaces) { + var anyNamespace; + if (showAny) { + anyNamespace = { + text: 'Any namespace', + id: null + }; + namespaces.unshift(anyNamespace); + namespaces.splice(1, 0, 'divider'); + } + return dataCallback(namespaces); }); - }); - } + }, + text: function(namespace) { + if (namespace.id == null) { + return namespace.text; + } else { + return namespace.kind + ": " + namespace.full_path; + } + }, + renderRow: this.renderRow, + clicked: this.onSelectItem + }); + } - return NamespaceSelects; - })(); -}).call(window); + onSelectItem(options) { + const { e } = options; + return e.preventDefault(); + } +} From fcc82ab601f1881558fc246f440f0ebe95bccab7 Mon Sep 17 00:00:00 2001 From: Winnie Hellmann Date: Thu, 26 Oct 2017 13:04:42 +0200 Subject: [PATCH 2/6] Inline onSelectItem of NamespaceSelect --- app/assets/javascripts/namespace_select.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/namespace_select.js b/app/assets/javascripts/namespace_select.js index de987b3d947..959b52c5608 100644 --- a/app/assets/javascripts/namespace_select.js +++ b/app/assets/javascripts/namespace_select.js @@ -3,7 +3,6 @@ import Api from './api'; export default class NamespaceSelect { constructor(opts) { - this.onSelectItem = this.onSelectItem.bind(this); var fieldName, showAny; this.dropdown = $(opts.dropdown); showAny = true; @@ -51,12 +50,10 @@ export default class NamespaceSelect { } }, renderRow: this.renderRow, - clicked: this.onSelectItem + clicked(options) { + const { e } = options; + return e.preventDefault(); + }, }); } - - onSelectItem(options) { - const { e } = options; - return e.preventDefault(); - } } From 6bae610c67040a71f19ad74125dc70bf6c33bc0b Mon Sep 17 00:00:00 2001 From: Winnie Hellmann Date: Thu, 26 Oct 2017 13:17:02 +0200 Subject: [PATCH 3/6] Replace showAny for NamespaceSelect by isFilter --- app/assets/javascripts/namespace_select.js | 9 +++------ app/views/admin/projects/index.html.haml | 2 +- app/views/admin/projects/show.html.haml | 2 +- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/namespace_select.js b/app/assets/javascripts/namespace_select.js index 959b52c5608..b53c1591bed 100644 --- a/app/assets/javascripts/namespace_select.js +++ b/app/assets/javascripts/namespace_select.js @@ -3,16 +3,13 @@ import Api from './api'; export default class NamespaceSelect { constructor(opts) { - var fieldName, showAny; + const isFilter = opts.dropdown.dataset.isFilter === 'true'; + var fieldName; this.dropdown = $(opts.dropdown); - showAny = true; fieldName = 'namespace_id'; if (this.dropdown.attr('data-field-name')) { fieldName = this.dropdown.data('fieldName'); } - if (this.dropdown.attr('data-show-any')) { - showAny = this.dropdown.data('showAny'); - } this.dropdown.glDropdown({ filterable: true, selectable: true, @@ -31,7 +28,7 @@ export default class NamespaceSelect { data: function(term, dataCallback) { return Api.namespaces(term, function(namespaces) { var anyNamespace; - if (showAny) { + if (isFilter) { anyNamespace = { text: 'Any namespace', id: null diff --git a/app/views/admin/projects/index.html.haml b/app/views/admin/projects/index.html.haml index 4d8754afdd2..c37d8ac45b9 100644 --- a/app/views/admin/projects/index.html.haml +++ b/app/views/admin/projects/index.html.haml @@ -14,7 +14,7 @@ = hidden_field_tag :namespace_id, params[:namespace_id] - namespace = Namespace.find(params[:namespace_id]) - toggle_text = "#{namespace.kind}: #{namespace.full_path}" - = dropdown_toggle(toggle_text, { toggle: 'dropdown' }, { toggle_class: 'js-namespace-select large' }) + = dropdown_toggle(toggle_text, { toggle: 'dropdown', is_filter: 'true' }, { toggle_class: 'js-namespace-select large' }) .dropdown-menu.dropdown-select.dropdown-menu-align-right = dropdown_title('Namespaces') = dropdown_filter("Search for Namespace") diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml index ab4165c0bf2..42f92079d85 100644 --- a/app/views/admin/projects/show.html.haml +++ b/app/views/admin/projects/show.html.haml @@ -115,7 +115,7 @@ = f.label :new_namespace_id, "Namespace", class: 'control-label' .col-sm-10 .dropdown - = dropdown_toggle('Search for Namespace', { toggle: 'dropdown', field_name: 'new_namespace_id', show_any: 'false' }, { toggle_class: 'js-namespace-select large' }) + = dropdown_toggle('Search for Namespace', { toggle: 'dropdown', field_name: 'new_namespace_id' }, { toggle_class: 'js-namespace-select large' }) .dropdown-menu.dropdown-select = dropdown_title('Namespaces') = dropdown_filter("Search for Namespace") From 46259f74d0571ce8607fddd74093b96284ef2489 Mon Sep 17 00:00:00 2001 From: Winnie Hellmann Date: Thu, 26 Oct 2017 13:37:58 +0200 Subject: [PATCH 4/6] Reduce eslint-disable from NamespaceSelect --- app/assets/javascripts/namespace_select.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/namespace_select.js b/app/assets/javascripts/namespace_select.js index b53c1591bed..69e83d784bf 100644 --- a/app/assets/javascripts/namespace_select.js +++ b/app/assets/javascripts/namespace_select.js @@ -1,4 +1,4 @@ -/* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, one-var, vars-on-top, one-var-declaration-per-line, comma-dangle, object-shorthand, no-else-return, prefer-template, quotes, prefer-arrow-callback, no-param-reassign, no-cond-assign, max-len */ +/* eslint-disable func-names, space-before-function-paren, no-var, comma-dangle, object-shorthand, no-else-return, prefer-template, quotes, prefer-arrow-callback, max-len */ import Api from './api'; export default class NamespaceSelect { From 34d53d2b5a150d10de3c84cb73f8a67f7fe297a7 Mon Sep 17 00:00:00 2001 From: Winnie Hellmann Date: Thu, 26 Oct 2017 14:47:17 +0200 Subject: [PATCH 5/6] Add failing test for https://gitlab.com/gitlab-org/gitlab-ce/issues/37277 --- spec/javascripts/namespace_select_spec.js | 65 +++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 spec/javascripts/namespace_select_spec.js diff --git a/spec/javascripts/namespace_select_spec.js b/spec/javascripts/namespace_select_spec.js new file mode 100644 index 00000000000..9d7625ca269 --- /dev/null +++ b/spec/javascripts/namespace_select_spec.js @@ -0,0 +1,65 @@ +import NamespaceSelect from '~/namespace_select'; + +describe('NamespaceSelect', () => { + beforeEach(() => { + spyOn($.fn, 'glDropdown'); + }); + + it('initializes glDropdown', () => { + const dropdown = document.createElement('div'); + + // eslint-disable-next-line no-new + new NamespaceSelect({ dropdown }); + + expect($.fn.glDropdown).toHaveBeenCalled(); + }); + + describe('as input', () => { + let glDropdownOptions; + + beforeEach(() => { + const dropdown = document.createElement('div'); + // eslint-disable-next-line no-new + new NamespaceSelect({ dropdown }); + glDropdownOptions = $.fn.glDropdown.calls.argsFor(0)[0]; + }); + + it('prevents click events', () => { + const dummyEvent = new Event('dummy'); + spyOn(dummyEvent, 'preventDefault'); + + glDropdownOptions.clicked({ e: dummyEvent }); + + expect(dummyEvent.preventDefault).toHaveBeenCalled(); + }); + }); + + describe('as filter', () => { + let glDropdownOptions; + + beforeEach(() => { + const dropdown = document.createElement('div'); + dropdown.dataset.isFilter = 'true'; + // eslint-disable-next-line no-new + new NamespaceSelect({ dropdown }); + glDropdownOptions = $.fn.glDropdown.calls.argsFor(0)[0]; + }); + + it('does not prevent click events', () => { + const dummyEvent = new Event('dummy'); + spyOn(dummyEvent, 'preventDefault'); + + glDropdownOptions.clicked({ e: dummyEvent }); + + expect(dummyEvent.preventDefault).not.toHaveBeenCalled(); + }); + + it('sets URL of dropdown items', () => { + const dummyNamespace = { id: 'eal' }; + + const itemUrl = glDropdownOptions.url(dummyNamespace); + + expect(itemUrl).toContain(`namespace_id=${dummyNamespace.id}`); + }); + }); +}); From 3a023c5ede3a8acbdb03c53ffa3580e5f0da46aa Mon Sep 17 00:00:00 2001 From: Winnie Hellmann Date: Thu, 26 Oct 2017 14:53:57 +0200 Subject: [PATCH 6/6] Make NamespaceSelect change URL when filtering --- app/assets/javascripts/namespace_select.js | 23 ++++++++++--------- .../winh-admin-projects-namespace-filter.yml | 5 ++++ 2 files changed, 17 insertions(+), 11 deletions(-) create mode 100644 changelogs/unreleased/winh-admin-projects-namespace-filter.yml diff --git a/app/assets/javascripts/namespace_select.js b/app/assets/javascripts/namespace_select.js index 69e83d784bf..1d496c64e53 100644 --- a/app/assets/javascripts/namespace_select.js +++ b/app/assets/javascripts/namespace_select.js @@ -1,16 +1,13 @@ /* eslint-disable func-names, space-before-function-paren, no-var, comma-dangle, object-shorthand, no-else-return, prefer-template, quotes, prefer-arrow-callback, max-len */ import Api from './api'; +import './lib/utils/url_utility'; export default class NamespaceSelect { constructor(opts) { const isFilter = opts.dropdown.dataset.isFilter === 'true'; - var fieldName; - this.dropdown = $(opts.dropdown); - fieldName = 'namespace_id'; - if (this.dropdown.attr('data-field-name')) { - fieldName = this.dropdown.data('fieldName'); - } - this.dropdown.glDropdown({ + const fieldName = opts.dropdown.dataset.fieldName || 'namespace_id'; + + $(opts.dropdown).glDropdown({ filterable: true, selectable: true, filterRemote: true, @@ -27,9 +24,8 @@ export default class NamespaceSelect { }, data: function(term, dataCallback) { return Api.namespaces(term, function(namespaces) { - var anyNamespace; if (isFilter) { - anyNamespace = { + const anyNamespace = { text: 'Any namespace', id: null }; @@ -48,8 +44,13 @@ export default class NamespaceSelect { }, renderRow: this.renderRow, clicked(options) { - const { e } = options; - return e.preventDefault(); + if (!isFilter) { + const { e } = options; + e.preventDefault(); + } + }, + url(namespace) { + return gl.utils.mergeUrlParams({ [fieldName]: namespace.id }, window.location.href); }, }); } diff --git a/changelogs/unreleased/winh-admin-projects-namespace-filter.yml b/changelogs/unreleased/winh-admin-projects-namespace-filter.yml new file mode 100644 index 00000000000..7e906f446b0 --- /dev/null +++ b/changelogs/unreleased/winh-admin-projects-namespace-filter.yml @@ -0,0 +1,5 @@ +--- +title: Make NamespaceSelect change URL when filtering +merge_request: 14888 +author: +type: fixed