Commit 4d3a0803 authored by Fatih Acet's avatar Fatih Acet

Merge branch 'dropdown-input-fix' into 'master'

Do not allow text input in dropdown while loading

## What does this MR do?
It moves the focus event of the text input of a filterable dropdown to after loading of options is complete. After the fix, the user cannot edit the while it's greyed out and loading.
## Are there points in the code the reviewer needs to double check?

## Why was this MR needed?
It fixes the bug in https://gitlab.com/gitlab-org/gitlab-ce/issues/23496.
## Screenshots (if relevant)
![https://gitlab.com/gitlab-org/gitlab-ce/uploads/a7bc2f0228fcde5a85cccb333b52f0e3/2016-10-18_12.00.54.gif](https://gitlab.com/gitlab-org/gitlab-ce/uploads/a7bc2f0228fcde5a85cccb333b52f0e3/2016-10-18_12.00.54.gif)

## What are the relevant issue numbers?
Fixes #23496

See merge request !7054
parents dcc699af 3acfbcae
...@@ -36,6 +36,7 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -36,6 +36,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` - Fix documents and comments on Build API `scope`
- Refactor email, use setter method instead AR callbacks for email attribute (Semyon Pupkov) - Refactor email, use setter method instead AR callbacks for email attribute (Semyon Pupkov)
- Shortened merge request modal to let clipboard button not overlap - 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 ## 8.13.2
- Fix builds dropdown overlapping bug !7124 - Fix builds dropdown overlapping bug !7124
......
...@@ -239,6 +239,7 @@ ...@@ -239,6 +239,7 @@
this.fullData = this.options.data; this.fullData = this.options.data;
currentIndex = -1; currentIndex = -1;
this.parseData(this.options.data); this.parseData(this.options.data);
this.focusTextInput();
} else { } else {
this.remote = new GitLabDropdownRemote(this.options.data, { this.remote = new GitLabDropdownRemote(this.options.data, {
dataType: this.options.dataType, dataType: this.options.dataType,
...@@ -247,6 +248,7 @@ ...@@ -247,6 +248,7 @@
return function(data) { return function(data) {
_this.fullData = data; _this.fullData = data;
_this.parseData(_this.fullData); _this.parseData(_this.fullData);
_this.focusTextInput();
if (_this.options.filterable && _this.filter && _this.filter.input) { if (_this.options.filterable && _this.filter && _this.filter.input) {
return _this.filter.input.trigger('input'); return _this.filter.input.trigger('input');
} }
...@@ -452,9 +454,8 @@ ...@@ -452,9 +454,8 @@
contentHtml = $('.dropdown-content', this.dropdown).html(); contentHtml = $('.dropdown-content', this.dropdown).html();
if (this.remote && contentHtml === "") { if (this.remote && contentHtml === "") {
this.remote.execute(); this.remote.execute();
} } else {
if (this.options.filterable) { this.focusTextInput();
this.filterInput.focus();
} }
if (this.options.showMenuAbove) { if (this.options.showMenuAbove) {
...@@ -691,6 +692,10 @@ ...@@ -691,6 +692,10 @@
return selectedObject; return selectedObject;
}; };
GitLabDropdown.prototype.focusTextInput = function() {
if (this.options.filterable) { this.filterInput.focus() }
}
GitLabDropdown.prototype.addInput = function(fieldName, value, selectedObject) { GitLabDropdown.prototype.addInput = function(fieldName, value, selectedObject) {
var $input; var $input;
// Create hidden input for form // Create hidden input for form
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
(() => { (() => {
const NON_SELECTABLE_CLASSES = '.divider, .separator, .dropdown-header, .dropdown-menu-empty-link'; 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 ITEM_SELECTOR = `.dropdown-content li:not(${NON_SELECTABLE_CLASSES})`;
const FOCUSED_ITEM_SELECTOR = `${ITEM_SELECTOR} a.is-focused`; const FOCUSED_ITEM_SELECTOR = `${ITEM_SELECTOR} a.is-focused`;
...@@ -17,6 +18,8 @@ ...@@ -17,6 +18,8 @@
ESC: 27 ESC: 27
}; };
let remoteCallback;
let navigateWithKeys = function navigateWithKeys(direction, steps, cb, i) { let navigateWithKeys = function navigateWithKeys(direction, steps, cb, i) {
i = i || 0; i = i || 0;
if (!i) direction = direction.toUpperCase(); if (!i) direction = direction.toUpperCase();
...@@ -33,18 +36,19 @@ ...@@ -33,18 +36,19 @@
} }
}; };
let remoteMock = function remoteMock(data, term, callback) {
remoteCallback = callback.bind({}, data);
}
describe('Dropdown', function describeDropdown() { describe('Dropdown', function describeDropdown() {
fixture.preload('gl_dropdown.html'); fixture.preload('gl_dropdown.html');
fixture.preload('projects.json'); fixture.preload('projects.json');
beforeEach(() => { function initDropDown(hasRemote, isFilterable) {
fixture.load('gl_dropdown.html');
this.dropdownContainerElement = $('.dropdown.inline');
this.dropdownMenuElement = $('.dropdown-menu', this.dropdownContainerElement);
this.projectsData = fixture.load('projects.json')[0];
this.dropdownButtonElement = $('#js-project-dropdown', this.dropdownContainerElement).glDropdown({ this.dropdownButtonElement = $('#js-project-dropdown', this.dropdownContainerElement).glDropdown({
selectable: true, selectable: true,
data: this.projectsData, filterable: isFilterable,
data: hasRemote ? remoteMock.bind({}, this.projectsData) : this.projectsData,
text: (project) => { text: (project) => {
(project.name_with_namespace || project.name); (project.name_with_namespace || project.name);
}, },
...@@ -52,6 +56,13 @@ ...@@ -52,6 +56,13 @@
project.id; 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(() => { afterEach(() => {
...@@ -60,6 +71,7 @@ ...@@ -60,6 +71,7 @@
}); });
it('should open on click', () => { it('should open on click', () => {
initDropDown.call(this, false);
expect(this.dropdownContainerElement).not.toHaveClass('open'); expect(this.dropdownContainerElement).not.toHaveClass('open');
this.dropdownButtonElement.click(); this.dropdownButtonElement.click();
expect(this.dropdownContainerElement).toHaveClass('open'); expect(this.dropdownContainerElement).toHaveClass('open');
...@@ -67,26 +79,27 @@ ...@@ -67,26 +79,27 @@
describe('that is open', () => { describe('that is open', () => {
beforeEach(() => { beforeEach(() => {
initDropDown.call(this, false, false);
this.dropdownButtonElement.click(); this.dropdownButtonElement.click();
}); });
it('should select a following item on DOWN keypress', () => { 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); let randomIndex = (Math.floor(Math.random() * (this.projectsData.length - 1)) + 0);
navigateWithKeys('down', randomIndex, () => { navigateWithKeys('down', randomIndex, () => {
expect($(FOCUSED_ITEM_SELECTOR, this.dropdownMenuElement).length).toBe(1); expect($(FOCUSED_ITEM_SELECTOR, this.$dropdownMenuElement).length).toBe(1);
expect($(`${ITEM_SELECTOR}:eq(${randomIndex}) a`, this.dropdownMenuElement)).toHaveClass('is-focused'); expect($(`${ITEM_SELECTOR}:eq(${randomIndex}) a`, this.$dropdownMenuElement)).toHaveClass('is-focused');
}); });
}); });
it('should select a previous item on UP keypress', () => { 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), () => { 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); let randomIndex = (Math.floor(Math.random() * (this.projectsData.length - 2)) + 0);
navigateWithKeys('up', randomIndex, () => { navigateWithKeys('up', randomIndex, () => {
expect($(FOCUSED_ITEM_SELECTOR, this.dropdownMenuElement).length).toBe(1); 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($(`${ITEM_SELECTOR}:eq(${((this.projectsData.length - 2) - randomIndex)}) a`, this.$dropdownMenuElement)).toHaveClass('is-focused');
}); });
}); });
}); });
...@@ -98,7 +111,7 @@ ...@@ -98,7 +111,7 @@
spyOn(Turbolinks, 'visit').and.stub(); spyOn(Turbolinks, 'visit').and.stub();
navigateWithKeys('enter', null, () => { navigateWithKeys('enter', null, () => {
expect(this.dropdownContainerElement).not.toHaveClass('open'); 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'); expect(link).toHaveClass('is-active');
let linkedLocation = link.attr('href'); let linkedLocation = link.attr('href');
if (linkedLocation && linkedLocation !== '#') expect(Turbolinks.visit).toHaveBeenCalledWith(linkedLocation); if (linkedLocation && linkedLocation !== '#') expect(Turbolinks.visit).toHaveBeenCalledWith(linkedLocation);
...@@ -116,5 +129,42 @@ ...@@ -116,5 +129,42 @@
expect(this.dropdownContainerElement).not.toHaveClass('open'); 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));
});
});
}); });
})(); })();
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment