Make sure $el contains on the el

Alternative solution to #3421 I think doing this check once on `_ensureElement` is better than within `attachHtml` And it makes `this.$el` more consistent and matches what happens in the region constructor
marionettejs · Aug 17, 2017 · 575bf47 · 575bf47
1 parent 5e6d5eb
commit 575bf47
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 20 deletions.
diff --git a/src/region.js b/src/region.js
@@ -133,6 +133,8 @@ const Region = MarionetteObject.extend({
     if (!_.isObject(this.el)) {
       this.$el = this.getEl(this.el);
       this.el = this.$el[0];
+      // Make sure the $el contains only the el
+      this.$el = this.Dom.getEl(this.el);
     }
 
     if (!this.$el || this.$el.length === 0) {
@@ -241,7 +243,7 @@ const Region = MarionetteObject.extend({
   // Override this method to change how the new view is appended to the `$el` that the
   // region is managing
   attachHtml(view) {
-    this.Dom.appendContents(this.el, view.el, {_$contents: view.$el});
+    this.Dom.appendContents(this.el, view.el, {_$el: this.$el, _$contents: view.$el});
   },
 
   // Destroy the current view, if there is one. If there is no current view, it does

diff --git a/test/unit/common/build-region.spec.js b/test/unit/common/build-region.spec.js
@@ -115,29 +115,46 @@ describe('Region', function() {
             expect(this.region.el).to.equal(this.fooSelector);
           });
 
-          describe('with `parentEl` also defined including the selector', function() {
-            beforeEach(function() {
-              this.setFixtures('<div id="parent"><div id="child">text</div></div>');
-              this.parentEl = $('#parent');
-              this.definition = _.defaults({parentEl: this.parentEl, el: '#child' }, this.definition);
-              this.region = this.view.addRegion(_.uniqueId('region_'),this.definition);
-            });
-
-            it('returns the jQuery(el)', function() {
-              expect(this.region.getEl(this.region.el).text()).to.equal($(this.region.el).text());
+          describe('with `parentEl` also defined', function() {
+            describe('including the selector', function() {
+              beforeEach(function() {
+                this.setFixtures('<div id="parent"><div id="child">text</div></div>');
+                this.parentEl = $('#parent');
+                this.definition = _.defaults({parentEl: this.parentEl, el: '#child' }, this.definition);
+                this.region = this.view.addRegion(_.uniqueId('region_'),this.definition);
+              });
+
+              it('returns the jQuery(el)', function() {
+                expect(this.region.getEl(this.region.el).text()).to.equal($(this.region.el).text());
+              });
             });
-          });
 
-          describe('with `parentEl` also defined excluding the selector', function() {
-            beforeEach(function() {
-              this.setFixtures('<div id="parent"></div><div id="not-child">text</div>');
-              this.parentEl = $('#parent');
-              this.definition = _.defaults({parentEl: this.parentEl, el: '#not-child' }, this.definition);
-              this.region = this.view.addRegion(_.uniqueId('region_'),this.definition);
+            describe('excluding the selector', function() {
+              beforeEach(function() {
+                this.setFixtures('<div id="parent"></div><div id="not-child">text</div>');
+                this.parentEl = $('#parent');
+                this.definition = _.defaults({parentEl: this.parentEl, el: '#not-child' }, this.definition);
+                this.region = this.view.addRegion(_.uniqueId('region_'),this.definition);
+              });
+
+              it('returns the jQuery(el)', function() {
+                expect(this.region.getEl(this.region.el).text()).to.not.equal($(this.region.el).text());
+              });
             });
 
-            it('returns the jQuery(el)', function() {
-              expect(this.region.getEl(this.region.el).text()).to.not.equal($(this.region.el).text());
+            describe('including multiple instances of the selector', function() {
+              beforeEach(function() {
+                this.setFixtures('<div id="parent"><div class="child">text</div><div class="child">text</div></div>');
+                this.parentEl = $('#parent');
+                this.definition = _.defaults({parentEl: this.parentEl, el: '.child' }, this.definition);
+                this.region = this.view.addRegion(_.uniqueId('region_'),this.definition);
+              });
+
+              it('should ensure a jQuery(el) of length 1', function() {
+                // calls _ensureElement
+                this.region.empty();
+                expect(this.region.$el.length).to.equal(1);
+              });
             });
           });
         });