From 45823c7a59c393b294f520bed9483e80c3d65295 Mon Sep 17 00:00:00 2001 From: Umer Farooq Date: Fri, 9 Dec 2016 21:07:12 -0500 Subject: [PATCH 1/3] feat(dropdown): make dropdown-append-to-body configurable Make dropdown-append-to-body accept a value which will be evaluated to determine if the menu should be appended to body. If no value is specified, or a non-false value is specified then the menu will be appended to body. If the value is `false` then the menu will not be appended to body. --- src/dropdown/docs/readme.md | 2 +- src/dropdown/dropdown.js | 7 +- src/dropdown/test/dropdown.spec.js | 124 +++++++++++++++++++++++------ 3 files changed, 107 insertions(+), 26 deletions(-) diff --git a/src/dropdown/docs/readme.md b/src/dropdown/docs/readme.md index 09b646d072..3026673ce2 100644 --- a/src/dropdown/docs/readme.md +++ b/src/dropdown/docs/readme.md @@ -25,7 +25,7 @@ Each of these parts need to be used as attribute directives. * `dropdown-append-to-body` B _(Default: `false`)_ - - Appends the inner dropdown-menu to the body element. + Appends the inner dropdown-menu to the body element if the attribute is present without a value, or with a non `false` value. * `is-open` $ diff --git a/src/dropdown/dropdown.js b/src/dropdown/dropdown.js index 7c5f805419..9e58d934f6 100644 --- a/src/dropdown/dropdown.js +++ b/src/dropdown/dropdown.js @@ -169,7 +169,12 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.multiMap', 'ui.bootstrap. } } - appendToBody = angular.isDefined($attrs.dropdownAppendToBody); + if (angular.isDefined($attrs.dropdownAppendToBody)) { + var appendToBodyValue = $parse($attrs.dropdownAppendToBody)(scope); + if (appendToBodyValue !== false) { + appendToBody = true; + } + } keynavEnabled = angular.isDefined($attrs.keyboardNav); if (appendToBody && !appendTo) { diff --git a/src/dropdown/test/dropdown.spec.js b/src/dropdown/test/dropdown.spec.js index dce7542ba8..c68e6365f3 100644 --- a/src/dropdown/test/dropdown.spec.js +++ b/src/dropdown/test/dropdown.spec.js @@ -215,38 +215,114 @@ describe('uib-dropdown', function() { }); describe('using dropdown-append-to-body', function() { - function dropdown() { - return $compile('
  • ')($rootScope); - } + describe('with no value', function() { + function dropdown() { + return $compile('
  • ')($rootScope); + } - beforeEach(function() { - element = dropdown(); - $document.find('body').append(element); - }); + beforeEach(function() { + element = dropdown(); + $document.find('body').append(element); + }); - afterEach(function() { - element.remove(); - }); + afterEach(function() { + element.remove(); + }); - it('adds the menu to the body', function() { - expect($document.find('#dropdown-menu').parent()[0]).toBe($document.find('body')[0]); - }); + it('adds the menu to the body', function() { + expect($document.find('#dropdown-menu').parent()[0]).toBe($document.find('body')[0]); + }); - it('focuses the dropdown element on close', function() { - var toggle = element.find('[uib-dropdown-toggle]'); - var menu = $document.find('#dropdown-menu a'); - toggle.trigger('click'); - menu.focus(); + it('focuses the dropdown element on close', function() { + var toggle = element.find('[uib-dropdown-toggle]'); + var menu = $document.find('#dropdown-menu a'); + toggle.trigger('click'); + menu.focus(); - menu.trigger('click'); + menu.trigger('click'); - expect(document.activeElement).toBe(toggle[0]); + expect(document.activeElement).toBe(toggle[0]); + }); + + it('removes the menu when the dropdown is removed', function() { + element.remove(); + $rootScope.$digest(); + expect($document.find('#dropdown-menu').length).toEqual(0); + }); }); - it('removes the menu when the dropdown is removed', function() { - element.remove(); - $rootScope.$digest(); - expect($document.find('#dropdown-menu').length).toEqual(0); + describe('with a value', function() { + function dropdown() { + return $compile('
  • ')($rootScope); + } + describe('that is not false', function() { + beforeEach(function() { + $rootScope.appendToBody = 'sure'; + + element = dropdown(); + $document.find('body').append(element); + }); + + afterEach(function() { + element.remove(); + }); + + it('adds the menu to the body', function() { + expect($document.find('#dropdown-menu').parent()[0]).toBe($document.find('body')[0]); + }); + + it('focuses the dropdown element on close', function() { + var toggle = element.find('[uib-dropdown-toggle]'); + var menu = $document.find('#dropdown-menu a'); + toggle.trigger('click'); + menu.focus(); + + menu.trigger('click'); + + expect(document.activeElement).toBe(toggle[0]); + }); + + it('removes the menu when the dropdown is removed', function() { + element.remove(); + $rootScope.$digest(); + expect($document.find('#dropdown-menu').length).toEqual(0); + }); + }); + + describe('that is false', function() { + beforeEach(function() { + $rootScope.appendToBody = false; + + element = dropdown(); + $document.find('body').append(element); + }); + + afterEach(function() { + element.remove(); + }); + + it('does not add the menu to the body', function() { + expect($document.find('#dropdown-menu').parent()[0]).not.toBe($document.find('body')[0]); + }); + + it('focuses the dropdown element on close', function() { + var toggle = element.find('[uib-dropdown-toggle]'); + var menu = $document.find('#dropdown-menu a'); + toggle.trigger('click'); + menu.focus(); + + menu.trigger('click'); + + expect(document.activeElement).toBe(toggle[0]); + }); + + it('removes the menu when the dropdown is removed', function() { + element.remove(); + $rootScope.$digest(); + expect($document.find('#dropdown-menu').length).toEqual(0); + }); + }); + }); }); From d218dc147c7ed3408e8319e0504d5ce493776471 Mon Sep 17 00:00:00 2001 From: Umer Farooq Date: Mon, 12 Dec 2016 20:53:25 -0500 Subject: [PATCH 2/3] feat(dropdown): append and remove menu when menu is opened or closed Only append and remove append-to and append-to-body menus when the menu is opened or closed. This allows for the values of append-to and append-to-body to be evaluated when the menu is toggled open, and also prevents littering of the DOM. --- src/dropdown/dropdown.js | 61 ++++--- src/dropdown/test/dropdown.spec.js | 256 ++++++++++++++++++++--------- 2 files changed, 217 insertions(+), 100 deletions(-) diff --git a/src/dropdown/dropdown.js b/src/dropdown/dropdown.js index 9e58d934f6..a3fc762cb6 100644 --- a/src/dropdown/dropdown.js +++ b/src/dropdown/dropdown.js @@ -144,8 +144,6 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.multiMap', 'ui.bootstrap. getIsOpen, setIsOpen = angular.noop, toggleInvoker = $attrs.onToggle ? $parse($attrs.onToggle) : angular.noop, - appendToBody = false, - appendTo = null, keynavEnabled = false, selectedOption = null, body = $document.find('body'); @@ -162,31 +160,7 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.multiMap', 'ui.bootstrap. }); } - if (angular.isDefined($attrs.dropdownAppendTo)) { - var appendToEl = $parse($attrs.dropdownAppendTo)(scope); - if (appendToEl) { - appendTo = angular.element(appendToEl); - } - } - - if (angular.isDefined($attrs.dropdownAppendToBody)) { - var appendToBodyValue = $parse($attrs.dropdownAppendToBody)(scope); - if (appendToBodyValue !== false) { - appendToBody = true; - } - } keynavEnabled = angular.isDefined($attrs.keyboardNav); - - if (appendToBody && !appendTo) { - appendTo = body; - } - - if (appendTo && self.dropdownMenu) { - appendTo.append(self.dropdownMenu); - $element.on('$destroy', function handleDestroyEvent() { - self.dropdownMenu.remove(); - }); - } }; this.toggle = function(open) { @@ -258,7 +232,42 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.multiMap', 'ui.bootstrap. } }; + function removeDropdownMenu() { + self.dropdownMenu.remove(); + } + scope.$watch('isOpen', function(isOpen, wasOpen) { + var appendTo = null, + appendToBody = false; + + if (angular.isDefined($attrs.dropdownAppendTo)) { + var appendToEl = $parse($attrs.dropdownAppendTo)(scope); + if (appendToEl) { + appendTo = angular.element(appendToEl); + } + } + + if (angular.isDefined($attrs.dropdownAppendToBody)) { + var appendToBodyValue = $parse($attrs.dropdownAppendToBody)(scope); + if (appendToBodyValue !== false) { + appendToBody = true; + } + } + + if (appendToBody && !appendTo) { + appendTo = body; + } + + if (appendTo && self.dropdownMenu) { + if (isOpen) { + appendTo.append(self.dropdownMenu); + $element.on('$destroy', removeDropdownMenu); + } else { + $element.off('$destroy', removeDropdownMenu); + removeDropdownMenu(); + } + } + if (appendTo && self.dropdownMenu) { var pos = $position.positionElements($element, self.dropdownMenu, 'bottom-left', true), css, diff --git a/src/dropdown/test/dropdown.spec.js b/src/dropdown/test/dropdown.spec.js index c68e6365f3..442e67dee1 100644 --- a/src/dropdown/test/dropdown.spec.js +++ b/src/dropdown/test/dropdown.spec.js @@ -229,25 +229,52 @@ describe('uib-dropdown', function() { element.remove(); }); - it('adds the menu to the body', function() { - expect($document.find('#dropdown-menu').parent()[0]).toBe($document.find('body')[0]); + it('does not add the menu to the body', function() { + expect($document.find('#dropdown-menu').parent()[0]).not.toBe($document.find('body')[0]); }); - it('focuses the dropdown element on close', function() { - var toggle = element.find('[uib-dropdown-toggle]'); - var menu = $document.find('#dropdown-menu a'); - toggle.trigger('click'); - menu.focus(); - - menu.trigger('click'); + describe('when toggled open', function() { + var toggle; + beforeEach(function() { + toggle = element.find('[uib-dropdown-toggle]'); + toggle.trigger('click'); + }); + it('adds the menu to the body', function() { + expect($document.find('#dropdown-menu').parent()[0]).toBe($document.find('body')[0]); + }); - expect(document.activeElement).toBe(toggle[0]); - }); + describe('when toggled closed', function() { + beforeEach(function() { + toggle.trigger('click'); + }); + it('removes the menu', function() { + expect($document.find('#dropdown-menu').length).toEqual(0); + }); + }); - it('removes the menu when the dropdown is removed', function() { - element.remove(); - $rootScope.$digest(); - expect($document.find('#dropdown-menu').length).toEqual(0); + describe('when closed by clicking on menu', function() { + var menu; + beforeEach(function() { + menu = $document.find('#dropdown-menu a'); + menu.focus(); + menu.trigger('click'); + }); + it('focuses the dropdown element on close', function() { + expect(document.activeElement).toBe(toggle[0]); + }); + it('removes the menu', function() { + expect($document.find('#dropdown-menu').length).toEqual(0); + }); + }); + describe('when the dropdown is removed', function() { + beforeEach(function() { + element.remove(); + $rootScope.$digest(); + }); + it('removes the menu', function() { + expect($document.find('#dropdown-menu').length).toEqual(0); + }); + }); }); }); @@ -266,26 +293,52 @@ describe('uib-dropdown', function() { afterEach(function() { element.remove(); }); - - it('adds the menu to the body', function() { - expect($document.find('#dropdown-menu').parent()[0]).toBe($document.find('body')[0]); - }); - - it('focuses the dropdown element on close', function() { - var toggle = element.find('[uib-dropdown-toggle]'); - var menu = $document.find('#dropdown-menu a'); - toggle.trigger('click'); - menu.focus(); - - menu.trigger('click'); - - expect(document.activeElement).toBe(toggle[0]); + it('does not add the menu to the body', function() { + expect($document.find('#dropdown-menu').parent()[0]).not.toBe($document.find('body')[0]); }); - it('removes the menu when the dropdown is removed', function() { - element.remove(); - $rootScope.$digest(); - expect($document.find('#dropdown-menu').length).toEqual(0); + describe('when toggled open', function() { + var toggle; + beforeEach(function() { + toggle = element.find('[uib-dropdown-toggle]'); + toggle.trigger('click'); + }); + it('adds the menu to the body', function() { + expect($document.find('#dropdown-menu').parent()[0]).toBe($document.find('body')[0]); + }); + + describe('when toggled closed', function() { + beforeEach(function() { + toggle.trigger('click'); + }); + it('removes the menu', function() { + expect($document.find('#dropdown-menu').length).toEqual(0); + }); + }); + + describe('when closed by clicking on menu', function() { + var menu; + beforeEach(function() { + menu = $document.find('#dropdown-menu a'); + menu.focus(); + menu.trigger('click'); + }); + it('focuses the dropdown element on close', function() { + expect(document.activeElement).toBe(toggle[0]); + }); + it('removes the menu', function() { + expect($document.find('#dropdown-menu').length).toEqual(0); + }); + }); + describe('when the dropdown is removed', function() { + beforeEach(function() { + element.remove(); + $rootScope.$digest(); + }); + it('removes the menu', function() { + expect($document.find('#dropdown-menu').length).toEqual(0); + }); + }); }); }); @@ -305,29 +358,55 @@ describe('uib-dropdown', function() { expect($document.find('#dropdown-menu').parent()[0]).not.toBe($document.find('body')[0]); }); - it('focuses the dropdown element on close', function() { - var toggle = element.find('[uib-dropdown-toggle]'); - var menu = $document.find('#dropdown-menu a'); - toggle.trigger('click'); - menu.focus(); - - menu.trigger('click'); - - expect(document.activeElement).toBe(toggle[0]); - }); - - it('removes the menu when the dropdown is removed', function() { - element.remove(); - $rootScope.$digest(); - expect($document.find('#dropdown-menu').length).toEqual(0); + describe('when toggled open', function() { + var toggle; + beforeEach(function() { + toggle = element.find('[uib-dropdown-toggle]'); + toggle.trigger('click'); + }); + it('does not add the menu to the body', function() { + expect($document.find('#dropdown-menu').parent()[0]).not.toBe($document.find('body')[0]); + }); + + describe('when toggled closed', function() { + beforeEach(function() { + toggle.trigger('click'); + }); + it('does not remove the menu', function() { + expect($document.find('#dropdown-menu').length).not.toEqual(0); + }); + }); + + describe('when closed by clicking on menu', function() { + var menu; + beforeEach(function() { + menu = $document.find('#dropdown-menu a'); + menu.focus(); + menu.trigger('click'); + }); + it('focuses the dropdown element on close', function() { + expect(document.activeElement).toBe(toggle[0]); + }); + it('does not removes the menu', function() { + expect($document.find('#dropdown-menu').length).not.toEqual(0); + }); + }); + describe('when the dropdown is removed', function() { + beforeEach(function() { + element.remove(); + $rootScope.$digest(); + }); + it('removes the menu', function() { + expect($document.find('#dropdown-menu').length).toEqual(0); + }); + }); }); }); - }); }); describe('using dropdown-append-to', function() { - var initialPage; + var initialPage, container; function dropdown() { return $compile('
  • ')($rootScope); @@ -336,7 +415,7 @@ describe('uib-dropdown', function() { beforeEach(function() { $document.find('body').append(angular.element('')); - $rootScope.appendTo = $document.find('#dropdown-container'); + $rootScope.appendTo = container = $document.find('#dropdown-container'); element = dropdown(); $document.find('body').append(element); @@ -347,35 +426,64 @@ describe('uib-dropdown', function() { $document.find('#dropdown-container').remove(); }); - it('appends to container', function() { - expect($document.find('#dropdown-menu').parent()[0].id).toBe('dropdown-container'); + it('does not add the menu to the container', function() { + expect($document.find('#dropdown-menu').parent()[0]).not.toBe(container[0]); }); - - it('toggles open class on container', function() { - var container = $document.find('#dropdown-container'); - - expect(container).not.toHaveClass('uib-dropdown-open'); - element.find('[uib-dropdown-toggle]').click(); - expect(container).toHaveClass('uib-dropdown-open'); - element.find('[uib-dropdown-toggle]').click(); + it('does not add open class on container', function() { expect(container).not.toHaveClass('uib-dropdown-open'); }); - it('focuses the dropdown element on close', function() { - var toggle = element.find('[uib-dropdown-toggle]'); - var menu = $document.find('#dropdown-menu a'); - toggle.trigger('click'); - menu.focus(); - - menu.trigger('click'); + describe('when toggled open', function() { + var toggle; + beforeEach(function() { + toggle = element.find('[uib-dropdown-toggle]'); + toggle.trigger('click'); + }); + it('adds the menu to the container', function() { + expect($document.find('#dropdown-menu').parent()[0]).toBe(container[0]); + }); + it('adds open class on container', function() { + expect(container).toHaveClass('uib-dropdown-open'); + }); - expect(document.activeElement).toBe(toggle[0]); - }); + describe('when toggled closed', function() { + beforeEach(function() { + toggle.trigger('click'); + }); + it('removes the menu', function() { + expect($document.find('#dropdown-menu').length).toEqual(0); + }); + it('removes open class from container', function() { + expect(container).not.toHaveClass('uib-dropdown-open'); + }); + }); - it('removes the menu when the dropdown is removed', function() { - element.remove(); - $rootScope.$digest(); - expect($document.find('#dropdown-menu').length).toEqual(0); + describe('when closed by clicking on menu', function() { + var menu; + beforeEach(function() { + menu = $document.find('#dropdown-menu a'); + menu.focus(); + menu.trigger('click'); + }); + it('focuses the dropdown element on close', function() { + expect(document.activeElement).toBe(toggle[0]); + }); + it('removes the menu', function() { + expect($document.find('#dropdown-menu').length).toEqual(0); + }); + it('removes open class from container', function() { + expect(container).not.toHaveClass('uib-dropdown-open'); + }); + }); + describe('when the dropdown is removed', function() { + beforeEach(function() { + element.remove(); + $rootScope.$digest(); + }); + it('removes the menu', function() { + expect($document.find('#dropdown-menu').length).toEqual(0); + }); + }); }); }); From 4577cffa3cafc9329873d0b8782a279ca187722c Mon Sep 17 00:00:00 2001 From: Umer Farooq Date: Thu, 15 Dec 2016 23:46:18 -0500 Subject: [PATCH 3/3] fix(dropdown): don't remove the dropdown-menu on close Instead of removing the dropdown-menu on close, append it back to the original element. --- src/dropdown/dropdown.js | 2 +- src/dropdown/test/dropdown.spec.js | 44 +++++++++++++++--------------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/dropdown/dropdown.js b/src/dropdown/dropdown.js index a3fc762cb6..312e880905 100644 --- a/src/dropdown/dropdown.js +++ b/src/dropdown/dropdown.js @@ -233,7 +233,7 @@ angular.module('ui.bootstrap.dropdown', ['ui.bootstrap.multiMap', 'ui.bootstrap. }; function removeDropdownMenu() { - self.dropdownMenu.remove(); + $element.append(self.dropdownMenu); } scope.$watch('isOpen', function(isOpen, wasOpen) { diff --git a/src/dropdown/test/dropdown.spec.js b/src/dropdown/test/dropdown.spec.js index 442e67dee1..17226b5c5e 100644 --- a/src/dropdown/test/dropdown.spec.js +++ b/src/dropdown/test/dropdown.spec.js @@ -247,8 +247,8 @@ describe('uib-dropdown', function() { beforeEach(function() { toggle.trigger('click'); }); - it('removes the menu', function() { - expect($document.find('#dropdown-menu').length).toEqual(0); + it('removes the menu from body', function() { + expect($document.find('#dropdown-menu').parent()[0]).not.toBe($document.find('body')[0]); }); }); @@ -262,8 +262,8 @@ describe('uib-dropdown', function() { it('focuses the dropdown element on close', function() { expect(document.activeElement).toBe(toggle[0]); }); - it('removes the menu', function() { - expect($document.find('#dropdown-menu').length).toEqual(0); + it('removes the menu from body', function() { + expect($document.find('#dropdown-menu').parent()[0]).not.toBe($document.find('body')[0]); }); }); describe('when the dropdown is removed', function() { @@ -271,8 +271,8 @@ describe('uib-dropdown', function() { element.remove(); $rootScope.$digest(); }); - it('removes the menu', function() { - expect($document.find('#dropdown-menu').length).toEqual(0); + it('removes the menu from body', function() { + expect($document.find('#dropdown-menu').parent()[0]).not.toBe($document.find('body')[0]); }); }); }); @@ -311,8 +311,8 @@ describe('uib-dropdown', function() { beforeEach(function() { toggle.trigger('click'); }); - it('removes the menu', function() { - expect($document.find('#dropdown-menu').length).toEqual(0); + it('removes the menu from body', function() { + expect($document.find('#dropdown-menu').parent()[0]).not.toBe($document.find('body')[0]); }); }); @@ -326,8 +326,8 @@ describe('uib-dropdown', function() { it('focuses the dropdown element on close', function() { expect(document.activeElement).toBe(toggle[0]); }); - it('removes the menu', function() { - expect($document.find('#dropdown-menu').length).toEqual(0); + it('removes the menu from body', function() { + expect($document.find('#dropdown-menu').parent()[0]).not.toBe($document.find('body')[0]); }); }); describe('when the dropdown is removed', function() { @@ -335,8 +335,8 @@ describe('uib-dropdown', function() { element.remove(); $rootScope.$digest(); }); - it('removes the menu', function() { - expect($document.find('#dropdown-menu').length).toEqual(0); + it('removes the menu from body', function() { + expect($document.find('#dropdown-menu').parent()[0]).not.toBe($document.find('body')[0]); }); }); }); @@ -387,8 +387,8 @@ describe('uib-dropdown', function() { it('focuses the dropdown element on close', function() { expect(document.activeElement).toBe(toggle[0]); }); - it('does not removes the menu', function() { - expect($document.find('#dropdown-menu').length).not.toEqual(0); + it('does not removes the menu from body', function() { + expect($document.find('#dropdown-menu').parent()[0]).not.toBe($document.find('body')[0]); }); }); describe('when the dropdown is removed', function() { @@ -396,8 +396,8 @@ describe('uib-dropdown', function() { element.remove(); $rootScope.$digest(); }); - it('removes the menu', function() { - expect($document.find('#dropdown-menu').length).toEqual(0); + it('removes the menu from body', function() { + expect($document.find('#dropdown-menu').parent()[0]).not.toBe($document.find('body')[0]); }); }); }); @@ -450,8 +450,8 @@ describe('uib-dropdown', function() { beforeEach(function() { toggle.trigger('click'); }); - it('removes the menu', function() { - expect($document.find('#dropdown-menu').length).toEqual(0); + it('removes the menu from the container', function() { + expect($document.find('#dropdown-menu').parent()[0]).not.toBe($document.find('body')[0]); }); it('removes open class from container', function() { expect(container).not.toHaveClass('uib-dropdown-open'); @@ -468,8 +468,8 @@ describe('uib-dropdown', function() { it('focuses the dropdown element on close', function() { expect(document.activeElement).toBe(toggle[0]); }); - it('removes the menu', function() { - expect($document.find('#dropdown-menu').length).toEqual(0); + it('removes the menu from the container', function() { + expect($document.find('#dropdown-menu').parent()[0]).not.toBe($document.find('body')[0]); }); it('removes open class from container', function() { expect(container).not.toHaveClass('uib-dropdown-open'); @@ -480,8 +480,8 @@ describe('uib-dropdown', function() { element.remove(); $rootScope.$digest(); }); - it('removes the menu', function() { - expect($document.find('#dropdown-menu').length).toEqual(0); + it('removes the menu from the container', function() { + expect($document.find('#dropdown-menu').parent()[0]).not.toBe($document.find('body')[0]); }); }); });