From 3acfbcaea0aef39e257b2f6668db361fef3ac43c Mon Sep 17 00:00:00 2001 From: Ido Leibovich Date: Sat, 22 Oct 2016 18:46:53 +0300 Subject: [PATCH] Do not allow text input in dropdown while loading While loading, the entire dropdown is greyed out. However, The focus is on the text input, so user can input text. Move the focus event to after loading finished to fix this. --- CHANGELOG.md | 1 + app/assets/javascripts/gl_dropdown.js | 11 +++- spec/javascripts/gl_dropdown_spec.js.es6 | 78 +++++++++++++++++++----- 3 files changed, 73 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index efa5a213570..22deca6af79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Fix documents and comments on Build API `scope` - Refactor email, use setter method instead AR callbacks for email attribute (Semyon Pupkov) - Shortened merge request modal to let clipboard button not overlap + - In all filterable drop downs, put input field in focus only after load is complete (Ido @leibo) ## 8.13.2 - Fix builds dropdown overlapping bug !7124 diff --git a/app/assets/javascripts/gl_dropdown.js b/app/assets/javascripts/gl_dropdown.js index 1d9f641836f..98e43c4d088 100644 --- a/app/assets/javascripts/gl_dropdown.js +++ b/app/assets/javascripts/gl_dropdown.js @@ -239,6 +239,7 @@ this.fullData = this.options.data; currentIndex = -1; this.parseData(this.options.data); + this.focusTextInput(); } else { this.remote = new GitLabDropdownRemote(this.options.data, { dataType: this.options.dataType, @@ -247,6 +248,7 @@ return function(data) { _this.fullData = data; _this.parseData(_this.fullData); + _this.focusTextInput(); if (_this.options.filterable && _this.filter && _this.filter.input) { return _this.filter.input.trigger('input'); } @@ -452,9 +454,8 @@ contentHtml = $('.dropdown-content', this.dropdown).html(); if (this.remote && contentHtml === "") { this.remote.execute(); - } - if (this.options.filterable) { - this.filterInput.focus(); + } else { + this.focusTextInput(); } if (this.options.showMenuAbove) { @@ -691,6 +692,10 @@ return selectedObject; }; + GitLabDropdown.prototype.focusTextInput = function() { + if (this.options.filterable) { this.filterInput.focus() } + } + GitLabDropdown.prototype.addInput = function(fieldName, value, selectedObject) { var $input; // Create hidden input for form diff --git a/spec/javascripts/gl_dropdown_spec.js.es6 b/spec/javascripts/gl_dropdown_spec.js.es6 index 685e662edd3..8ba238018cd 100644 --- a/spec/javascripts/gl_dropdown_spec.js.es6 +++ b/spec/javascripts/gl_dropdown_spec.js.es6 @@ -7,6 +7,7 @@ (() => { const NON_SELECTABLE_CLASSES = '.divider, .separator, .dropdown-header, .dropdown-menu-empty-link'; + const SEARCH_INPUT_SELECTOR = '.dropdown-input-field'; const ITEM_SELECTOR = `.dropdown-content li:not(${NON_SELECTABLE_CLASSES})`; const FOCUSED_ITEM_SELECTOR = `${ITEM_SELECTOR} a.is-focused`; @@ -17,6 +18,8 @@ ESC: 27 }; + let remoteCallback; + let navigateWithKeys = function navigateWithKeys(direction, steps, cb, i) { i = i || 0; if (!i) direction = direction.toUpperCase(); @@ -33,18 +36,19 @@ } }; + let remoteMock = function remoteMock(data, term, callback) { + remoteCallback = callback.bind({}, data); + } + describe('Dropdown', function describeDropdown() { fixture.preload('gl_dropdown.html'); fixture.preload('projects.json'); - beforeEach(() => { - fixture.load('gl_dropdown.html'); - this.dropdownContainerElement = $('.dropdown.inline'); - this.dropdownMenuElement = $('.dropdown-menu', this.dropdownContainerElement); - this.projectsData = fixture.load('projects.json')[0]; + function initDropDown(hasRemote, isFilterable) { this.dropdownButtonElement = $('#js-project-dropdown', this.dropdownContainerElement).glDropdown({ selectable: true, - data: this.projectsData, + filterable: isFilterable, + data: hasRemote ? remoteMock.bind({}, this.projectsData) : this.projectsData, text: (project) => { (project.name_with_namespace || project.name); }, @@ -52,6 +56,13 @@ project.id; } }); + } + + beforeEach(() => { + fixture.load('gl_dropdown.html'); + this.dropdownContainerElement = $('.dropdown.inline'); + this.$dropdownMenuElement = $('.dropdown-menu', this.dropdownContainerElement); + this.projectsData = fixture.load('projects.json')[0]; }); afterEach(() => { @@ -60,6 +71,7 @@ }); it('should open on click', () => { + initDropDown.call(this, false); expect(this.dropdownContainerElement).not.toHaveClass('open'); this.dropdownButtonElement.click(); expect(this.dropdownContainerElement).toHaveClass('open'); @@ -67,26 +79,27 @@ describe('that is open', () => { beforeEach(() => { + initDropDown.call(this, false, false); this.dropdownButtonElement.click(); }); it('should select a following item on DOWN keypress', () => { - expect($(FOCUSED_ITEM_SELECTOR, this.dropdownMenuElement).length).toBe(0); + expect($(FOCUSED_ITEM_SELECTOR, this.$dropdownMenuElement).length).toBe(0); let randomIndex = (Math.floor(Math.random() * (this.projectsData.length - 1)) + 0); navigateWithKeys('down', randomIndex, () => { - expect($(FOCUSED_ITEM_SELECTOR, this.dropdownMenuElement).length).toBe(1); - expect($(`${ITEM_SELECTOR}:eq(${randomIndex}) a`, this.dropdownMenuElement)).toHaveClass('is-focused'); + expect($(FOCUSED_ITEM_SELECTOR, this.$dropdownMenuElement).length).toBe(1); + expect($(`${ITEM_SELECTOR}:eq(${randomIndex}) a`, this.$dropdownMenuElement)).toHaveClass('is-focused'); }); }); it('should select a previous item on UP keypress', () => { - expect($(FOCUSED_ITEM_SELECTOR, this.dropdownMenuElement).length).toBe(0); + expect($(FOCUSED_ITEM_SELECTOR, this.$dropdownMenuElement).length).toBe(0); navigateWithKeys('down', (this.projectsData.length - 1), () => { - expect($(FOCUSED_ITEM_SELECTOR, this.dropdownMenuElement).length).toBe(1); + expect($(FOCUSED_ITEM_SELECTOR, this.$dropdownMenuElement).length).toBe(1); let randomIndex = (Math.floor(Math.random() * (this.projectsData.length - 2)) + 0); navigateWithKeys('up', randomIndex, () => { - expect($(FOCUSED_ITEM_SELECTOR, this.dropdownMenuElement).length).toBe(1); - expect($(`${ITEM_SELECTOR}:eq(${((this.projectsData.length - 2) - randomIndex)}) a`, this.dropdownMenuElement)).toHaveClass('is-focused'); + expect($(FOCUSED_ITEM_SELECTOR, this.$dropdownMenuElement).length).toBe(1); + expect($(`${ITEM_SELECTOR}:eq(${((this.projectsData.length - 2) - randomIndex)}) a`, this.$dropdownMenuElement)).toHaveClass('is-focused'); }); }); }); @@ -98,7 +111,7 @@ spyOn(Turbolinks, 'visit').and.stub(); navigateWithKeys('enter', null, () => { expect(this.dropdownContainerElement).not.toHaveClass('open'); - let link = $(`${ITEM_SELECTOR}:eq(${randomIndex}) a`, this.dropdownMenuElement); + let link = $(`${ITEM_SELECTOR}:eq(${randomIndex}) a`, this.$dropdownMenuElement); expect(link).toHaveClass('is-active'); let linkedLocation = link.attr('href'); if (linkedLocation && linkedLocation !== '#') expect(Turbolinks.visit).toHaveBeenCalledWith(linkedLocation); @@ -116,5 +129,42 @@ expect(this.dropdownContainerElement).not.toHaveClass('open'); }); }); + + describe('opened and waiting for a remote callback', () => { + beforeEach(() => { + initDropDown.call(this, true, true); + this.dropdownButtonElement.click(); + }); + + it('should not focus search input while remote task is not complete', ()=> { + expect($(document.activeElement)).not.toEqual($(SEARCH_INPUT_SELECTOR)); + remoteCallback(); + expect($(document.activeElement)).toEqual($(SEARCH_INPUT_SELECTOR)); + }); + + it('should focus search input after remote task is complete', ()=> { + remoteCallback(); + expect($(document.activeElement)).toEqual($(SEARCH_INPUT_SELECTOR)); + }); + + it('should focus on input when opening for the second time', ()=> { + remoteCallback(); + this.dropdownContainerElement.trigger({ + type: 'keyup', + which: ARROW_KEYS.ESC, + keyCode: ARROW_KEYS.ESC + }); + this.dropdownButtonElement.click(); + expect($(document.activeElement)).toEqual($(SEARCH_INPUT_SELECTOR)); + }); + }); + + describe('input focus with array data', () => { + it('should focus input when passing array data to drop down', ()=> { + initDropDown.call(this, false, true); + this.dropdownButtonElement.click(); + expect($(document.activeElement)).toEqual($(SEARCH_INPUT_SELECTOR)); + }); + }); }); })();