Commit b638e45c authored by Alfredo Sumaran's avatar Alfredo Sumaran

Merge branch '30951-start-discussion-toggle-clicking-divider-causes-ui-break' into 'master'

Resolve "start discussion toggle clicking divider causes UI break"

Closes #30951

See merge request !10715
parents 28c63ce3 16e291b9
...@@ -2,10 +2,12 @@ const DATA_TRIGGER = 'data-dropdown-trigger'; ...@@ -2,10 +2,12 @@ const DATA_TRIGGER = 'data-dropdown-trigger';
const DATA_DROPDOWN = 'data-dropdown'; const DATA_DROPDOWN = 'data-dropdown';
const SELECTED_CLASS = 'droplab-item-selected'; const SELECTED_CLASS = 'droplab-item-selected';
const ACTIVE_CLASS = 'droplab-item-active'; const ACTIVE_CLASS = 'droplab-item-active';
const IGNORE_CLASS = 'droplab-item-ignore';
export { export {
DATA_TRIGGER, DATA_TRIGGER,
DATA_DROPDOWN, DATA_DROPDOWN,
SELECTED_CLASS, SELECTED_CLASS,
ACTIVE_CLASS, ACTIVE_CLASS,
IGNORE_CLASS,
}; };
/* eslint-disable */ /* eslint-disable */
import utils from './utils'; import utils from './utils';
import { SELECTED_CLASS } from './constants'; import { SELECTED_CLASS, IGNORE_CLASS } from './constants';
var DropDown = function(list) { var DropDown = function(list) {
this.currentIndex = 0; this.currentIndex = 0;
...@@ -36,6 +36,7 @@ Object.assign(DropDown.prototype, { ...@@ -36,6 +36,7 @@ Object.assign(DropDown.prototype, {
clickEvent: function(e) { clickEvent: function(e) {
if (e.target.tagName === 'UL') return; if (e.target.tagName === 'UL') return;
if (e.target.classList.contains(IGNORE_CLASS)) return;
var selected = utils.closest(e.target, 'LI'); var selected = utils.closest(e.target, 'LI');
if (!selected) return; if (!selected) return;
......
...@@ -564,3 +564,7 @@ ...@@ -564,3 +564,7 @@
color: $gl-text-color-secondary; color: $gl-text-color-secondary;
} }
} }
.droplab-item-ignore {
pointer-events: none;
}
...@@ -16,7 +16,7 @@ ...@@ -16,7 +16,7 @@
%p %p
Add a general comment to this #{noteable_name}. Add a general comment to this #{noteable_name}.
%li.divider %li.divider.droplab-item-ignore
%li#discussion{ data: { value: 'DiscussionNote', 'submit-text' => 'Start discussion', 'close-text' => "Start discussion & close #{noteable_name}", 'reopen-text' => "Start discussion & reopen #{noteable_name}" } } %li#discussion{ data: { value: 'DiscussionNote', 'submit-text' => 'Start discussion', 'close-text' => "Start discussion & close #{noteable_name}", 'reopen-text' => "Start discussion & reopen #{noteable_name}" } }
%a{ href: '#' } %a{ href: '#' }
......
...@@ -26,4 +26,10 @@ describe('constants', function () { ...@@ -26,4 +26,10 @@ describe('constants', function () {
expect(constants.ACTIVE_CLASS).toBe('droplab-item-active'); expect(constants.ACTIVE_CLASS).toBe('droplab-item-active');
}); });
}); });
describe('IGNORE_CLASS', function () {
it('should be `droplab-item-ignore`', function() {
expect(constants.IGNORE_CLASS).toBe('droplab-item-ignore');
});
});
}); });
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
import DropDown from '~/droplab/drop_down'; import DropDown from '~/droplab/drop_down';
import utils from '~/droplab/utils'; import utils from '~/droplab/utils';
import { SELECTED_CLASS } from '~/droplab/constants'; import { SELECTED_CLASS, IGNORE_CLASS } from '~/droplab/constants';
describe('DropDown', function () { describe('DropDown', function () {
describe('class constructor', function () { describe('class constructor', function () {
...@@ -128,9 +128,10 @@ describe('DropDown', function () { ...@@ -128,9 +128,10 @@ describe('DropDown', function () {
describe('clickEvent', function () { describe('clickEvent', function () {
beforeEach(function () { beforeEach(function () {
this.classList = jasmine.createSpyObj('classList', ['contains']);
this.list = { dispatchEvent: () => {} }; this.list = { dispatchEvent: () => {} };
this.dropdown = { hide: () => {}, list: this.list, addSelectedClass: () => {} }; this.dropdown = { hide: () => {}, list: this.list, addSelectedClass: () => {} };
this.event = { preventDefault: () => {}, target: {} }; this.event = { preventDefault: () => {}, target: { classList: this.classList } };
this.customEvent = {}; this.customEvent = {};
this.closestElement = {}; this.closestElement = {};
...@@ -140,6 +141,7 @@ describe('DropDown', function () { ...@@ -140,6 +141,7 @@ describe('DropDown', function () {
spyOn(this.event, 'preventDefault'); spyOn(this.event, 'preventDefault');
spyOn(window, 'CustomEvent').and.returnValue(this.customEvent); spyOn(window, 'CustomEvent').and.returnValue(this.customEvent);
spyOn(utils, 'closest').and.returnValues(this.closestElement, undefined); spyOn(utils, 'closest').and.returnValues(this.closestElement, undefined);
this.classList.contains.and.returnValue(false);
DropDown.prototype.clickEvent.call(this.dropdown, this.event); DropDown.prototype.clickEvent.call(this.dropdown, this.event);
}); });
...@@ -164,15 +166,35 @@ describe('DropDown', function () { ...@@ -164,15 +166,35 @@ describe('DropDown', function () {
expect(window.CustomEvent).toHaveBeenCalledWith('click.dl', jasmine.any(Object)); expect(window.CustomEvent).toHaveBeenCalledWith('click.dl', jasmine.any(Object));
}); });
it('should call .classList.contains checking for IGNORE_CLASS', function () {
expect(this.classList.contains).toHaveBeenCalledWith(IGNORE_CLASS);
});
it('should call .dispatchEvent with the customEvent', function () { it('should call .dispatchEvent with the customEvent', function () {
expect(this.list.dispatchEvent).toHaveBeenCalledWith(this.customEvent); expect(this.list.dispatchEvent).toHaveBeenCalledWith(this.customEvent);
}); });
describe('if the target is a UL element', function () { describe('if the target is a UL element', function () {
beforeEach(function () { beforeEach(function () {
this.event = { preventDefault: () => {}, target: { tagName: 'UL' } }; this.event = { preventDefault: () => {}, target: { tagName: 'UL', classList: this.classList } };
spyOn(this.event, 'preventDefault');
utils.closest.calls.reset();
DropDown.prototype.clickEvent.call(this.dropdown, this.event);
});
it('should return immediately', function () {
expect(utils.closest).not.toHaveBeenCalled();
});
});
describe('if the target has the IGNORE_CLASS class', function () {
beforeEach(function () {
this.event = { preventDefault: () => {}, target: { tagName: 'LI', classList: this.classList } };
spyOn(this.event, 'preventDefault'); spyOn(this.event, 'preventDefault');
this.classList.contains.and.returnValue(true);
utils.closest.calls.reset(); utils.closest.calls.reset();
DropDown.prototype.clickEvent.call(this.dropdown, this.event); DropDown.prototype.clickEvent.call(this.dropdown, this.event);
......
...@@ -73,9 +73,15 @@ shared_examples 'discussion comments' do |resource_name| ...@@ -73,9 +73,15 @@ shared_examples 'discussion comments' do |resource_name|
expect(page).not_to have_selector menu_selector expect(page).not_to have_selector menu_selector
end end
it 'clicking the ul padding should not change the text' do it 'clicking the ul padding or divider should not change the text' do
find(menu_selector).trigger 'click' find(menu_selector).trigger 'click'
expect(page).to have_selector menu_selector
expect(find(dropdown_selector)).to have_content 'Comment'
find("#{menu_selector} .divider").trigger 'click'
expect(page).to have_selector menu_selector
expect(find(dropdown_selector)).to have_content 'Comment' expect(find(dropdown_selector)).to have_content 'Comment'
end end
......
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