From 061dd23c89db317ff65239ee68c2f192ed4db749 Mon Sep 17 00:00:00 2001 From: Paul Falgout Date: Mon, 29 Apr 2019 01:17:26 +0900 Subject: [PATCH] WIP Next Region --- src/application.js | 24 +- src/collection-view.js | 2 +- src/common/build-region.js | 37 --- src/common/normalize-region.js | 19 ++ src/mixins/regions.js | 118 ++++----- src/mixins/ui.js | 47 +--- src/region.js | 173 ++++--------- src/view.js | 22 +- test/unit/common/build-region.spec.js | 326 ------------------------- test/unit/region.spec.js | 193 +++------------ test/unit/view.child-views.spec.js | 58 +---- test/unit/view.dynamic-regions.spec.js | 48 +--- 12 files changed, 214 insertions(+), 853 deletions(-) delete mode 100644 src/common/build-region.js create mode 100644 src/common/normalize-region.js delete mode 100644 test/unit/common/build-region.spec.js diff --git a/src/application.js b/src/application.js index facbc7d3dc..8820131204 100644 --- a/src/application.js +++ b/src/application.js @@ -3,7 +3,7 @@ import _ from 'underscore'; import extend from './utils/extend'; -import buildRegion from './common/build-region'; +import normalizeRegion from './common/normalize-region'; import CommonMixin from './mixins/common'; import DestroyMixin from './mixins/destroy'; import RadioMixin from './mixins/radio'; @@ -50,11 +50,25 @@ _.extend(Application.prototype, CommonMixin, DestroyMixin, RadioMixin, { if (!region) { return; } - const defaults = { - regionClass: this.regionClass - }; + this._region = this._buildRegion(region); + }, + + _buildRegion(definition) { + if (definition instanceof Region) { + return definition; + } + + if (_.isFunction(definition)) { + return this._buildRegion(definition.call(this)); + } + + const options = normalizeRegion(definition, this.regionClass); + + const RegionClass = options.regionClass; + + delete options.regionClass; - this._region = buildRegion(region, defaults); + return new RegionClass(options); }, getRegion() { diff --git a/src/collection-view.js b/src/collection-view.js index 272092aeb9..b23e3669a5 100644 --- a/src/collection-view.js +++ b/src/collection-view.js @@ -74,7 +74,7 @@ const CollectionView = Backbone.View.extend({ const $emptyEl = this.$container || this.$el; if (this._emptyRegion && !this._emptyRegion.isDestroyed()) { - this._emptyRegion._setElement($emptyEl[0]); + this._emptyRegion.setElement($emptyEl[0]); return this._emptyRegion; } diff --git a/src/common/build-region.js b/src/common/build-region.js deleted file mode 100644 index 02c46c65fc..0000000000 --- a/src/common/build-region.js +++ /dev/null @@ -1,37 +0,0 @@ -import _ from 'underscore'; -import MarionetteError from '../utils/error'; -import Region from '../region'; - -// return the region instance from the definition -export default function(definition, defaults) { - if (definition instanceof Region) { - return definition; - } - - if (_.isString(definition)) { - return buildRegionFromObject(defaults, { el: definition }); - } - - if (_.isFunction(definition)) { - return buildRegionFromObject(defaults, { regionClass: definition }); - } - - if (_.isObject(definition)) { - return buildRegionFromObject(defaults, definition); - } - - throw new MarionetteError({ - message: 'Improper region configuration type.', - url: 'marionette.region.html#defining-regions' - }); -} - -function buildRegionFromObject(defaults, definition) { - const options = _.extend({}, defaults, definition); - - const RegionClass = options.regionClass - - delete options.regionClass; - - return new RegionClass(options); -} diff --git a/src/common/normalize-region.js b/src/common/normalize-region.js new file mode 100644 index 0000000000..8750e62188 --- /dev/null +++ b/src/common/normalize-region.js @@ -0,0 +1,19 @@ +import _ from 'underscore'; +import MarionetteError from '../utils/error'; + +// return the region instance from the definition +export default function(definition, regionClass) { + if (_.isString(definition)) { + definition = { el: definition }; + } + + if (!_.isString(definition.el)) { + console.log(definition); + throw new MarionetteError({ + message: 'Improper region configuration type.', + url: 'marionette.region.html#defining-regions' + }); + } + + return _.extend({ regionClass }, definition); +} diff --git a/src/mixins/regions.js b/src/mixins/regions.js index 0a728c7a39..0884e631de 100644 --- a/src/mixins/regions.js +++ b/src/mixins/regions.js @@ -1,8 +1,16 @@ import _ from 'underscore'; import _invoke from '../utils/invoke'; -import buildRegion from '../common/build-region'; +import normalizeRegion from '../common/normalize-region'; import Region from '../region'; +// Performant startsWith +function startsWithUi(el) { + return el[0] === '@' && + el[1] === 'u' && + el[2] === 'i' && + el[3] === '.'; +} + // MixinOptions // - regions // - regionClass @@ -10,96 +18,79 @@ import Region from '../region'; export default { regionClass: Region, - // Internal method to initialize the regions that have been defined in a - // `regions` attribute on this View. + // Internal method to initialize the regions attributes _initRegions() { - // init regions hash - this.regions = this.regions || {}; + // init regions instance hash this._regions = {}; - - this.addRegions(_.result(this, 'regions')); + // init regions definition hash + this.regions = _.extend({}, _.result(this, 'regions')); }, - // Internal method to re-initialize all of the regions by updating - // the `el` that they point to - _reInitRegions() { - _invoke(this._regions, 'reset'); - }, + // Adds or rebinds all regions associated with the view + _bindRegions() { + const regions = this.addRegions(_.extend({}, this.regions)); - // Add a single region, by name, to the View - addRegion(name, definition) { - const regions = {}; - regions[name] = definition; - return this.addRegions(regions)[name]; + this.triggerMethod('bind:regions', this, regions); }, - // Add multiple regions as a {name: definition, name2: def2} object literal - addRegions(regions) { - // If there's nothing to add, stop here. - if (_.isEmpty(regions)) { - return; + // Finds an existing ui with `@ui.` or finds the `el` on the DOM + _findEl(el) { + if (_.isString(el) && startsWithUi(el)) { + return this.ui[el.slice(4)]; } - // Normalize region selectors hash to allow - // a user to use the @ui. syntax. - regions = this.normalizeUIValues(regions, 'el'); - - // Add the regions definitions to the regions property - this.regions = _.extend({}, this.regions, regions); - - return this._addRegions(regions); - }, - - // internal method to build and add regions - _addRegions(regionDefinitions) { - const defaults = { - regionClass: this.regionClass, - parentEl: _.partial(_.result, this, 'el') - }; - - return _.reduce(regionDefinitions, (regions, definition, name) => { - regions[name] = buildRegion(definition, defaults); - this._addRegion(regions[name], name); - return regions; - }, {}); + return this.Dom.findEl(this.el, el); }, - _addRegion(region, name) { - this.triggerMethod('before:add:region', this, name, region); + _buildRegion(name, RegionClass, options) { + const region = new RegionClass(options); region._parentView = this; region._name = name; this._regions[name] = region; - this.triggerMethod('add:region', this, name, region); + return region; }, - // Remove a single region from the View, by name - removeRegion(name) { - const region = this._regions[name]; + // Add a single region, by name, to the View + addRegion(name, definition) { + const currentRegion = this.getRegion(name); + definition = normalizeRegion(definition, this.regionClass); - this._removeRegion(region, name); + this.regions[name] = definition; - return region; - }, + const options = _.extend({}, definition); + options.el = this._findEl(options.el); - // Remove all regions from the View - removeRegions() { - const regions = this._getRegions(); + if (currentRegion) { + return currentRegion.setElement(options.el); + } - _.each(this._regions, this._removeRegion.bind(this)); + const RegionClass = options.regionClass; + delete options.regionClass; - return regions; + return this._buildRegion(name, RegionClass, options); }, - _removeRegion(region, name) { - this.triggerMethod('before:remove:region', this, name, region); + // Add multiple region definitions + addRegions(regionDefinitions) { + return _.map(regionDefinitions, (definition, name) => { + return this.addRegion(name, definition); + }); + }, - region.destroy(); + // Remove a single region from the View, by name + removeRegion(name) { + return this.getRegion(name).destroy(); + }, - this.triggerMethod('remove:region', this, name, region); + // Remove all regions from the View + removeRegions() { + const regions = this._getRegions(); + _invoke(regions, 'destroy'); + return regions; }, // Called in a region's destroy @@ -134,7 +125,7 @@ export default { }, _getRegions() { - return _.clone(this._regions); + return _.extend({}, this._regions); }, // Get all regions @@ -158,5 +149,4 @@ export default { getChildView(name) { return this.getRegion(name).currentView; } - }; diff --git a/src/mixins/ui.js b/src/mixins/ui.js index 83ce3af541..c0cecc3de0 100644 --- a/src/mixins/ui.js +++ b/src/mixins/ui.js @@ -1,15 +1,4 @@ import _ from 'underscore'; -// allows for the use of the @ui. syntax within -// a given key for triggers and events -// swaps the @ui with the associated selector. -// Returns a new, non-mutated, parsed events hash. -const normalizeUIKeys = function(hash, ui) { - return _.reduce(hash, (memo, val, key) => { - const normalizedKey = normalizeUIString(key, ui); - memo[normalizedKey] = val; - return memo; - }, {}); -}; const uiRegEx = /@ui\.[a-zA-Z-_$0-9]*/g; @@ -21,30 +10,19 @@ const normalizeUIString = function(uiString, ui) { }); }; -// allows for the use of the @ui. syntax within -// a given value for regions -// swaps the @ui with the associated selector -const normalizeUIValues = function(hash, ui, property) { - _.each(hash, (val, key) => { - if (_.isString(val)) { - hash[key] = normalizeUIString(val, ui); - } else if (val) { - const propertyVal = val[property]; - if (_.isString(propertyVal)) { - val[property] = normalizeUIString(propertyVal, ui); - } - } - }); - return hash; -}; - export default { - // normalize the keys of passed hash with the views `ui` selectors. - // `{"@ui.foo": "bar"}` + // allows for the use of the @ui. syntax within + // a given key for triggers and events + // swaps the @ui with the associated selector. + // Returns a new, non-mutated, parsed events hash. normalizeUIKeys(hash) { const uiBindings = this._getUIBindings(); - return normalizeUIKeys(hash, uiBindings); + return _.reduce(hash, (memo, val, key) => { + const normalizedKey = normalizeUIString(key, uiBindings); + memo[normalizedKey] = val; + return memo; + }, {}); }, // normalize the passed string with the views `ui` selectors. @@ -54,13 +32,6 @@ export default { return normalizeUIString(uiString, uiBindings); }, - // normalize the values of passed hash with the views `ui` selectors. - // `{foo: "@ui.bar"}` - normalizeUIValues(hash, property) { - const uiBindings = this._getUIBindings(); - return normalizeUIValues(hash, uiBindings, property); - }, - _getUIBindings() { const uiBindings = _.result(this, '_uiBindings'); return uiBindings || _.result(this, 'ui'); diff --git a/src/region.js b/src/region.js index 131199ef3a..e339495670 100644 --- a/src/region.js +++ b/src/region.js @@ -14,8 +14,8 @@ import DomApi, { setDomApi } from './config/dom'; const classErrorName = 'RegionError'; const ClassOptions = [ - 'allowMissingEl', - 'parentEl', + 'currentView', + 'el', 'replaceElement' ]; @@ -24,13 +24,8 @@ const Region = function(options) { this.cid = _.uniqueId(this.cidPrefix); - // getOption necessary because options.el may be passed as undefined - this._initEl = this.el = this.getOption('el'); - - // Handle when this.el is passed in as a $ wrapped element. - this.el = this.el instanceof Backbone.$ ? this.el[0] : this.el; - - this.$el = this._getEl(this.el); + this.$el = this.getEl(this.el); + this.el = this.$el[0]; this.initialize.apply(this, arguments); }; @@ -52,12 +47,48 @@ _.extend(Region.prototype, CommonMixin, { // This is a noop method intended to be overridden initialize() {}, + // Override this method to change how the region finds the DOM element that it manages. Return + // a jQuery selector object scoped to a provided parent el or the document if none exists. + getEl(el) { + return this.Dom.getEl(el); + }, + + // Set the `el` of the region and move any current view to the new `el`. + setElement(el) { + const $el = this.getEl(el); + + if ($el[0] === this.el) { return this; } + + const replaceElement = this._isReplaced; + + this._restoreEl(); + + this.$el = $el; + this.el = $el[0]; + + if (this.currentView) { + this._ensureElement(); + + this._attachView(this.currentView, { replaceElement }); + } + + return this; + }, + + _ensureElement() { + if (!this.el) { + throw new MarionetteError({ + name: classErrorName, + message: `A region el is required. Region cid:${this.cid}`, + url: 'marionette.region.html#additional-options' + }); + } + }, + // Displays a view instance inside of the region. If necessary handles calling the `render` // method for you. Reads content directly from the `el` attribute. show(view, options) { - if (!this._ensureElement(options)) { - return; - } + this._ensureElement(); view = this._getView(view, options); @@ -95,56 +126,6 @@ _.extend(Region.prototype, CommonMixin, { return this; }, - _getEl(el) { - if (!el) { - throw new MarionetteError({ - name: classErrorName, - message: 'An "el" must be specified for a region.', - url: 'marionette.region.html#additional-options' - }); - } - - return this.getEl(el); - }, - - _setEl() { - this.$el = this._getEl(this.el); - - if (this.$el.length) { - this.el = this.$el[0]; - } - - // Make sure the $el contains only the el - if (this.$el.length > 1) { - this.$el = this.Dom.getEl(this.el); - } - }, - - // Set the `el` of the region and move any current view to the new `el`. - _setElement(el) { - if (el === this.el) { return this; } - - const shouldReplace = this._isReplaced; - - this._restoreEl(); - - this.el = el; - - this._setEl(); - - if (this.currentView) { - const view = this.currentView; - - if (shouldReplace) { - this._replaceEl(view); - } else { - this.attachHtml(view); - } - } - - return this; - }, - _setupChildView(view) { monitorViewEvents(view); @@ -170,7 +151,7 @@ _.extend(Region.prototype, CommonMixin, { }, _isElAttached() { - return this.Dom.hasEl(this.Dom.getDocumentEl(this.el), this.el); + return !!this.el && this.Dom.hasEl(this.Dom.getDocumentEl(this.el), this.el); }, _attachView(view, { replaceElement } = {}) { @@ -196,27 +177,6 @@ _.extend(Region.prototype, CommonMixin, { view._isShown = true; }, - _ensureElement(options = {}) { - if (!_.isObject(this.el)) { - this._setEl(); - } - - if (!this.$el || this.$el.length === 0) { - const allowMissingEl = typeof options.allowMissingEl === 'undefined' ? !!_.result(this, 'allowMissingEl') : !!options.allowMissingEl; - - if (allowMissingEl) { - return false; - } else { - throw new MarionetteError({ - name: classErrorName, - message: `An "el" must exist in DOM for this region ${this.cid}`, - url: 'marionette.region.html#additional-options' - }); - } - } - return true; - }, - _getView(view) { if (!view) { throw new MarionetteError({ @@ -259,18 +219,6 @@ _.extend(Region.prototype, CommonMixin, { return { template }; }, - // Override this method to change how the region finds the DOM element that it manages. Return - // a jQuery selector object scoped to a provided parent el or the document if none exists. - getEl(el) { - const context = _.result(this, 'parentEl'); - - if (context && _.isString(el)) { - return this.Dom.findEl(context, el); - } - - return this.Dom.getEl(el); - }, - _replaceEl(view) { // Always restore the el to ensure the regions el is present before replacing this._restoreEl(); @@ -285,17 +233,11 @@ _.extend(Region.prototype, CommonMixin, { // Restore the region's element in the DOM. _restoreEl() { // There is nothing to replace - if (!this._isReplaced) { + if (!this._isReplaced || !this.currentView) { return; } - const view = this.currentView; - - if (!view) { - return; - } - - this._detachView(view); + this._detachView(this.currentView); this._isReplaced = false; }, @@ -318,14 +260,15 @@ _.extend(Region.prototype, CommonMixin, { // Destroy the current view, if there is one. If there is no current view, // it will detach any html inside the region's `el`. - empty(options = { allowMissingEl: true }) { + empty() { + if (!this.el) { return this; } + const view = this.currentView; // If there is no view in the region we should only detach current html if (!view) { - if (this._ensureElement(options)) { - this.detachHtml(); - } + this.detachHtml(); + return this; } @@ -422,18 +365,6 @@ _.extend(Region.prototype, CommonMixin, { return !!this.currentView; }, - // Reset the region by destroying any existing view and clearing out the cached `$el`. - // The next time a view is shown via this region, the region will re-query the DOM for - // the region's `el`. - reset(options) { - this.empty(options); - - this.el = this._initEl; - - delete this.$el; - return this; - }, - _isDestroyed: false, isDestroyed() { @@ -448,7 +379,7 @@ _.extend(Region.prototype, CommonMixin, { this.triggerMethod('before:destroy', this, options); this._isDestroyed = true; - this.reset(options); + this.empty(); if (this._name) { this._parentView._removeReferences(this._name); diff --git a/src/view.js b/src/view.js index 7cb1c3c546..7f88f6d67a 100644 --- a/src/view.js +++ b/src/view.js @@ -57,6 +57,8 @@ const View = Backbone.View.extend({ // if an el was previously defined. If so, the view might be // rendered or attached on setElement. setElement() { + const wasRendered = this._isRendered; + Backbone.View.prototype.setElement.apply(this, arguments); this._isRendered = this.Dom.hasContents(this.el); @@ -64,6 +66,11 @@ const View = Backbone.View.extend({ if (this._isRendered) { this.bindUIElements(); + this._bindRegions(); + } + + if (!wasRendered && (this._isRendered || this.getTemplate() === false)) { + this.triggerMethod('ready', this); } return this; @@ -78,18 +85,19 @@ const View = Backbone.View.extend({ this.triggerMethod('before:render', this); - // If this is not the first render call, then we need to - // re-initialize the `el` for each region - if (this._isRendered) { - this._reInitRegions(); - } - + const wasRendered = this._isRendered; this._renderTemplate(template); + this._isRendered = true; + this.bindUIElements(); + this._bindRegions(); - this._isRendered = true; this.triggerMethod('render', this); + if (!wasRendered && this._isRendered) { + this.triggerMethod('ready', this); + } + return this; }, diff --git a/test/unit/common/build-region.spec.js b/test/unit/common/build-region.spec.js deleted file mode 100644 index e4441a9c15..0000000000 --- a/test/unit/common/build-region.spec.js +++ /dev/null @@ -1,326 +0,0 @@ -import View from '../../../src/view'; -import Region from '../../../src/region'; - -describe('Region', function() { - describe('.buildRegion', function() { - let DefaultRegionClass; - let view; - let fooSelector; - let barSelector; - let BarRegion; - let BazRegion; - - beforeEach(function() { - - DefaultRegionClass = Region.extend(); - - view = new View({ - template: _.noop, - regionClass: DefaultRegionClass - }); - - fooSelector = '#foo-region'; - - barSelector = '#bar-region'; - BarRegion = Region.extend({el: barSelector}); - - BazRegion = Region.extend(); - }); - - describe('with a selector string', function() { - let region; - - beforeEach(function() { - region = view.addRegion(_.uniqueId('region_'), fooSelector); - }); - - it('uses the default region class', function() { - expect(region).to.be.an.instanceof(DefaultRegionClass); - }); - - it('uses the selector', function() { - expect(region.el).to.equal(fooSelector); - }); - }); - - describe('with a region class', function() { - describe('with `el` defined', function() { - let region; - - beforeEach(function() { - region = view.addRegion(_.uniqueId('region_'), BarRegion); - }); - - it('uses the passed in region class', function() { - expect(region).to.be.an.instanceof(BarRegion); - }); - - it('uses the defined el', function() { - expect(region.el).to.equal(barSelector); - }); - }); - - describe('without `el` defined', function() { - let buildRegion; - - beforeEach(function() { - buildRegion = function() { - view.addRegion(_.uniqueId('region_'), BazRegion); - }; - }); - - it('throws a `NoElError`', function() { - expect(buildRegion).to.throw('An "el" must be specified for a region.'); - }); - }); - }); - - describe('with an object literal', function() { - describe('with `el` defined', function() { - describe('when el is a selector string', function() { - let definition; - let region; - - beforeEach(function() { - definition = {el: fooSelector}; - region = view.addRegion(_.uniqueId('region_'),definition); - }); - - it('uses the default region class', function() { - expect(region).to.be.an.instanceof(DefaultRegionClass); - }); - - it('uses the el', function() { - expect(region.el).to.equal(fooSelector); - }); - - describe('with `parentEl` also defined', function() { - describe('including the selector', function() { - beforeEach(function() { - this.setFixtures('
text
'); - const parentEl = $('#parent'); - definition = _.defaults({parentEl: parentEl, el: '#child' }, definition); - region = view.addRegion(_.uniqueId('region_'), definition); - }); - - it('returns the jQuery(el)', function() { - expect(region.getEl(region.el).text()).to.equal($(region.el).text()); - }); - }); - - describe('excluding the selector', function() { - beforeEach(function() { - this.setFixtures('
text
'); - const parentEl = $('#parent'); - definition = _.defaults({parentEl: parentEl, el: '#not-child' }, definition); - region = view.addRegion(_.uniqueId('region_'), definition); - }); - - it('returns the jQuery(el)', function() { - expect(region.getEl(region.el).text()).to.not.equal($(region.el).text()); - }); - }); - - describe('including multiple instances of the selector', function() { - beforeEach(function() { - this.setFixtures('
text
text
'); - const parentEl = $('#parent'); - definition = _.defaults({parentEl: parentEl, el: '.child' }, definition); - region = view.addRegion(_.uniqueId('region_'), definition); - }); - - it('should ensure a jQuery(el) of length 1', function() { - // calls _ensureElement - region.empty(); - expect(region.$el.length).to.equal(1); - }); - }); - }); - }); - - describe('when el is an HTML node', function() { - let el; - let definition; - let region; - - beforeEach(function() { - el = $('
')[0]; - new DefaultRegionClass({el: el}); - definition = {el: el}; - region = view.addRegion(_.uniqueId('region_'), definition); - }); - - it('uses the default region class', function() { - expect(region).to.be.an.instanceof(DefaultRegionClass); - }); - - it('uses the el', function() { - expect(region.el).to.equal(el); - }); - - describe('with `parentEl` also defined', function() { - beforeEach(function() { - const parentEl = $('
'); - definition = _.defaults({parentEl: parentEl}, definition); - region = view.addRegion(_.uniqueId('region_'), definition); - }); - - it('returns the jQuery(el)', function() { - expect(region.getEl(el)).to.deep.equal($(el)); - }); - - }); - }); - - describe('when el is a jQuery object', function() { - let el; - let region; - - beforeEach(function() { - el = $('
'); - new DefaultRegionClass({el: el}); - const definition = {el: el}; - region = view.addRegion(_.uniqueId('region_'), definition); - }); - - it('uses the default region class', function() { - expect(region).to.be.an.instanceof(DefaultRegionClass); - }); - - it('uses the el', function() { - expect(region.el).to.equal(el[0]); - }); - }); - }); - - describe('when el is an empty jQuery object', function() { - let buildRegion; - - beforeEach(function() { - const el = $('i-am-not-real'); - const definition = {el: el}; - - buildRegion = function() { - view.addRegion(_.uniqueId('region_'), definition); - }; - }); - - it('throws a `NoElError`', function() { - expect(buildRegion).to.throw('An "el" must be specified for a region.'); - }); - }); - - describe('with `regionClass` defined', function() { - describe('with `el` also defined', function() { - let el; - let region1; - let region2; - let region3; - - beforeEach(function() { - const $el = $('
'); - el = $el[0]; - - const baseDefinition = {regionClass: BazRegion}; - const region1Definition = _.defaults({el: fooSelector}, baseDefinition); - const region2Definition = _.defaults({el: el}, baseDefinition); - const region3Definition = _.defaults({el: $el}, baseDefinition); - - region1 = view.addRegion(_.uniqueId('region_'), region1Definition); - region2 = view.addRegion(_.uniqueId('region_'), region2Definition); - region3 = view.addRegion(_.uniqueId('region_'), region3Definition); - }); - - it('uses the region class', function() { - expect(region1).to.be.an.instanceof(BazRegion); - expect(region2).to.be.an.instanceof(BazRegion); - expect(region3).to.be.an.instanceof(BazRegion); - }); - - it('uses the el', function() { - expect(region1.el).to.equal(fooSelector); - expect(region2.el).to.equal(el); - expect(region3.el).to.equal(el); - }); - }); - - describe('without `selector` or `el` defined on `regionConfig`', function() { - describe('with `el` defined on `regionClass`', function() { - let region; - - beforeEach(function() { - const definition = {regionClass: BarRegion}; - region = view.addRegion(_.uniqueId('region_'), definition); - }); - - it('uses the region class', function() { - expect(region).to.be.an.instanceof(BarRegion); - }); - }); - - describe('without `el` defined on `regionClass`', function() { - let buildRegion; - - beforeEach(function() { - const definition = {regionClass: BazRegion}; - - buildRegion = function() { - view.addRegion(_.uniqueId('region_'), definition); - }; - }); - - it('throws a `NoElError`', function() { - expect(buildRegion).to.throw('An "el" must be specified for a region.'); - }); - }); - }); - }); - - describe('with additional region options', function() { - let region; - - beforeEach(function() { - const definition = { - el: fooSelector, - regionClass: BazRegion, - myRegionOption: 42, - myOtherRegionOption: 'foobar' - }; - - region = view.addRegion(_.uniqueId('region_'), definition); - }); - - it('it sets the region options', function() { - expect(region.getOption('myRegionOption')).to.equal(42); - expect(region.getOption('myOtherRegionOption')).to.equal('foobar'); - }); - }); - }); - - describe('with a instantiated region', function() { - let region; - - beforeEach(function() { - region = view.addRegion(_.uniqueId('region_'), new BarRegion()); - }); - - it('uses the region class', function() { - expect(region).to.be.an.instanceof(BarRegion); - }); - }); - - describe('with a missing regionConfig', function() { - let buildRegion; - - beforeEach(function() { - buildRegion = function() { - view.addRegion(_.uniqueId('region_')); - }; - }); - - it('throws an error', function() { - expect(buildRegion).to.throw('Improper region configuration type.'); - }); - }); - }); -}); diff --git a/test/unit/region.spec.js b/test/unit/region.spec.js index 58df421711..a8653d1147 100644 --- a/test/unit/region.spec.js +++ b/test/unit/region.spec.js @@ -8,19 +8,10 @@ import CollectionView from '../../src/collection-view'; describe('region', function() { 'use strict'; - describe('when creating a new region and no configuration has been provided', function() { - it('should throw an exception saying an "el" is required', function() { - expect(function() { - return new Region(); - }).to.throw('An "el" must be specified for a region.'); - }); - }); - describe('when passing an el DOM reference in directly', function() { let el; let customRegion; let optionRegion; - let optionRegionJquery; beforeEach(function() { this.setFixtures('
'); @@ -31,19 +22,12 @@ describe('region', function() { }))(); optionRegion = new Region({el: el}); - - optionRegionJquery = new Region({el: $(el)}); }); it('should not have been replaced', function() { expect(customRegion.isReplaced()).to.be.false; }); - it('should work when el is passed in as an option', function() { - expect(optionRegionJquery.$el[0]).to.equal(el); - expect(optionRegionJquery.el).to.equal(el); - }); - it('should handle when the el option is passed in as a jquery selector', function() { expect(optionRegion.$el[0]).to.equal(el); }); @@ -77,7 +61,6 @@ describe('region', function() { describe('when creating a new region and the "el" does not exist in DOM', function() { let MyRegion; let MyView; - let myView; beforeEach(function() { MyRegion = Region.extend({ @@ -89,52 +72,25 @@ describe('region', function() { $(this.el).html('some content'); } }); - myView = new MyView(); this.setFixtures('
'); }); describe('when showing a view', function() { - describe('when allowMissingEl is not set', function() { - let region; - - beforeEach(function() { - region = new MyRegion(); - }); - - it('should throw an exception saying an "el" doesnt exist in DOM', function() { - expect(function() { - region.show(new MyView()); - }.bind(this)).to.throw('An "el" must exist in DOM for this region ' + region.cid); - }); + let region; - it('should not have a view', function() { - expect(region.hasView()).to.be.false; - }); + beforeEach(function() { + region = new MyRegion(); }); - describe('when allowMissingEl is set', function() { - let region; - - beforeEach(function() { - region = new MyRegion({allowMissingEl: true}); - }); - - it('should not throw an exception', function() { - expect(function() { - region.show(new MyView()); - }.bind(this)).not.to.throw(); - }); - - it('should not have a view', function() { - expect(region.hasView()).to.be.false; - }); + it('should throw an exception saying an "el" doesnt exist in DOM', function() { + expect(function() { + region.show(new MyView()); + }.bind(this)).to.throw('A region el is required. Region cid:' + region.cid); + }); - it('should not render the view', function() { - this.sinon.spy(myView, 'render'); - region.show(myView); - expect(myView.render).not.to.have.been.called; - }); + it('should not have a view', function() { + expect(region.hasView()).to.be.false; }); }); }); @@ -156,30 +112,28 @@ describe('region', function() { }); it('should return the region', function() { - expect(region._setElement(twoEl)).to.equal(region); + expect(region.setElement(twoEl)).to.equal(region); }); it('should set the el', function() { region.show(new TestView()); - region._setElement(twoEl); + region.setElement(twoEl); expect(region.el).to.equal(twoEl); }); - it('should set the $el', function() { - region.show(new TestView()); - region._setElement(twoEl); - expect(region.$el[0]).to.equal($(twoEl)[0]); + it('should not throw an error if the `el` is not specified', function() { + expect(region.setElement.bind(region)).to.not.throw(); }); - it('should throw an error if the `el` is not specified', function() { - expect(region._setElement.bind(region)).to.throw(); + it('should throw an error if the `el` is not specified with currentView', function() { + region.currentView = true; + expect(region.setElement.bind(region)).to.throw(); }); describe('when setting the `el` to the same element', function() { it('should not requery the el', function() { this.sinon.spy(region, 'getEl'); - expect(region._setElement(oneEl)).to.equal(region); - expect(region.getEl).to.not.be.called; + expect(region.setElement(oneEl)).to.equal(region); }); }); @@ -187,7 +141,7 @@ describe('region', function() { it('should replace the el of the region with the view el', function() { const view = new TestView(); region.show(view, { replaceElement: true }); - region._setElement(twoEl); + region.setElement(twoEl); expect($('#region1')).to.be.lengthOf(1); expect($('#view')).to.be.lengthOf(1); expect($('#region2')).to.be.lengthOf(0); @@ -198,7 +152,7 @@ describe('region', function() { it('should attach the view html to the region', function() { const view = new TestView(); region.show(view, { replaceElement: false }); - region._setElement(twoEl); + region.setElement(twoEl); expect($('#region1')).to.be.lengthOf(1); expect($('#region1 #view')).to.be.lengthOf(0); expect($('#region2 #view')).to.be.lengthOf(1); @@ -212,7 +166,7 @@ describe('region', function() { beforeEach(function() { this.setFixtures('
'); myRegion = new Region({ - el: '#region' + el: $('#region')[0] }); myRegion.show(_.template('Hello World!')); @@ -1031,22 +985,6 @@ describe('region', function() { }); }); - describe('when initializing a region and passing an "el" option', function() { - let el; - let region; - - beforeEach(function() { - el = '#foo'; - region = new Region({ - el: el - }); - }); - - it('should manage the specified el', function() { - expect(region.el).to.equal(el); - }); - }); - describe('when creating a region instance with an initialize method', function() { let expectedOptions; let MyRegion; @@ -1104,12 +1042,14 @@ describe('region', function() { let region; beforeEach(function() { - itemView = new View(); - itemView.render = this.sinon.stub(); - itemView.addRegions({ - MyRegion: '#region', - anotherRegion: '#region2' + itemView = new View({ + regions: { + MyRegion: '#region', + anotherRegion: '#region2' + }, + template: false }); + this.sinon.spy(itemView, 'render'); region = itemView._regions.MyRegion; }); @@ -1124,37 +1064,6 @@ describe('region', function() { }); }); - describe('when resetting a region', function() { - let region; - - beforeEach(function() { - this.setFixtures('
'); - - region = new Region({ - el: '#region' - }); - - this.sinon.spy(region, 'empty'); - - region.show(new View({ template: false })); - - this.sinon.spy(region, 'reset'); - region.reset(); - }); - - it('should not hold on to the regions previous "el"', function() { - expect(region.$el).not.to.exist; - }); - - it('should empty any existing view', function() { - expect(region.empty).to.have.been.called; - }); - - it('should return the region', function() { - expect(region.reset).to.have.returned(region); - }); - }); - describe('when destroying a region', function() { let region; @@ -1165,14 +1074,14 @@ describe('region', function() { el: '#region' }); - this.sinon.spy(region, 'reset'); + this.sinon.spy(region, 'empty'); this.sinon.spy(region, 'destroy'); region.destroy(); }); - it('should reset the region', function() { - expect(region.reset).to.have.been.called; + it('should empty the region', function() { + expect(region.empty).to.have.been.called; }); it('should return the region', function() { @@ -1180,10 +1089,10 @@ describe('region', function() { }); describe('when the region is already destroyed', function() { - it('should not reset the region', function() { - region.reset.resetHistory(); + it('should not empty the region', function() { + region.empty.resetHistory(); region.destroy(); - expect(region.reset).to.not.have.been.called; + expect(region.empty).to.not.have.been.called; }); it('should return the region', function() { @@ -1317,30 +1226,6 @@ describe('region', function() { }); }); - describe('when calling "_ensureElement"', function() { - let region; - - beforeEach(function() { - region = new Region({ - el: '#region' - }); - }); - - it('should prefer passed options over initial options', function() { - region.allowMissingEl = false; - - expect(region._ensureElement({allowMissingEl: true})).to.be.false; - }); - - it('should fallback to initial options when not passed options', function() { - region.allowMissingEl = false; - - expect(function() { - region._ensureElement(); - }.bind(this)).to.throw; - }); - }); - // This is a terrible example of an edge-case where something related to the view's destroy // may also want to empty the same region. describe('when emptying a region destroys a view that empties the same region', function() { @@ -1386,15 +1271,5 @@ describe('region', function() { region.empty(); expect(region.$el.html()).to.eql(''); }); - - // In the future, hopefully allowMissingEl can default to true - describe('when no el exists while passing allowMissingEl: false', function() { - it('should throw an error', function() { - region = new MyRegion(); - expect(function() { - region.empty({ allowMissingEl: false }); - }).to.throw('An "el" must exist in DOM for this region ' + region.cid); - }); - }); }); }); diff --git a/test/unit/view.child-views.spec.js b/test/unit/view.child-views.spec.js index 5e260e7f7a..559f65a438 100644 --- a/test/unit/view.child-views.spec.js +++ b/test/unit/view.child-views.spec.js @@ -33,7 +33,7 @@ describe('layoutView', function() { this.ViewNoDefaultRegion = this.View.extend({ regions: { regionOne: { - selector: '#regionOne', + el: '#regionOne', regionClass: this.CustomRegion1 }, regionTwo: '#regionTwo' @@ -470,8 +470,8 @@ describe('layoutView', function() { this.layoutView.render(); }); - it('should call empty twice', function() { - expect(this.region.empty).to.have.been.calledThrice; + it('should call empty once', function() { + expect(this.region.empty).to.be.calledOnce; }); describe('and the views "render" function is bound to an event in the "initialize" function', function() { @@ -522,7 +522,7 @@ describe('layoutView', function() { war: '.craft', is: { regionClass: this.CustomRegion, - selector: '#a-fun-game' + el: '#a-fun-game' } }; @@ -550,6 +550,7 @@ describe('layoutView', function() { }); it('should set custom region classes', function() { + this.layoutView.render(); expect(this.CustomRegion).to.have.been.called; }); }); @@ -637,53 +638,4 @@ describe('layoutView', function() { }); }); }); - - describe('manipulating regions', function() { - beforeEach(function() { - this.beforeAddRegionSpy = this.sinon.spy(); - this.addRegionSpy = this.sinon.spy(); - this.beforeRegionRemoveSpy = this.sinon.spy(); - this.removeRegionSpy = this.sinon.spy(); - - this.Layout = Marionette.View.extend({ - template: _.noop, - onBeforeAddRegion: this.beforeAddRegionSpy, - onAddRegion: this.addRegionSpy, - onBeforeRemoveRegion: this.beforeRegionRemoveSpy, - onRemoveRegion: this.removeRegionSpy - }); - - this.layout = new this.Layout(); - - this.regionName = 'myRegion'; - this.layout.addRegion(this.regionName, '.region-selector'); - }); - - it('should trigger correct region add events', function() { - expect(this.beforeAddRegionSpy) - .to.have.been.calledOnce - .and.calledOn(this.layout) - .and.calledWith(this.layout, this.regionName); - - expect(this.addRegionSpy) - .to.have.been.calledOnce - .and.calledOn(this.layout) - .and.calledWith(this.layout, this.regionName); - }); - - it('should trigger correct region remove events', function() { - this.layout.removeRegion(this.regionName); - - expect(this.beforeRegionRemoveSpy) - .to.have.been.calledOnce - .and.calledOn(this.layout) - .and.calledWith(this.layout, this.regionName); - - expect(this.removeRegionSpy) - .to.have.been.calledOnce - .and.calledOn(this.layout) - .and.calledWith(this.layout, this.regionName); - }); - }); - }); diff --git a/test/unit/view.dynamic-regions.spec.js b/test/unit/view.dynamic-regions.spec.js index 4e020cb45d..26208de3aa 100644 --- a/test/unit/view.dynamic-regions.spec.js +++ b/test/unit/view.dynamic-regions.spec.js @@ -14,22 +14,12 @@ describe('itemView - dynamic regions', function() { describe('when adding a region to a layoutView, after it has been rendered', function() { beforeEach(function() { - this.MyView = Marionette.View.extend({ - onAddRegion: function() {}, - onBeforeAddRegion: function() {} - }); + this.MyView = Marionette.View.extend(); this.layoutView = new this.MyView({ template: this.template }); - this.beforeAddHandler = this.sinon.spy(); - this.addHandler = this.sinon.spy(); - this.onBeforeAddSpy = this.sinon.spy(this.layoutView, 'onBeforeAddRegion'); - this.onAddSpy = this.sinon.spy(this.layoutView, 'onAddRegion'); - this.layoutView.on('before:add:region', this.beforeAddHandler); - this.layoutView.on('add:region', this.addHandler); - this.layoutView.render(); this.region = this.layoutView.addRegion('foo', '#foo'); @@ -43,7 +33,10 @@ describe('itemView - dynamic regions', function() { }); it('should add the region definition to the regions property', function() { - expect(this.layoutView.regions.foo).to.equal('#foo'); + expect(this.layoutView.regions.foo).to.eql({ + el: '#foo', + regionClass: this.layoutView.regionClass + }); }); it('should set the parent of the region to the layoutView', function() { @@ -53,16 +46,6 @@ describe('itemView - dynamic regions', function() { it('should be able to show a view in the region', function() { expect(this.layoutView.getRegion('foo').$el.children().length).to.equal(1); }); - - it('should trigger a before:add:region event', function() { - expect(this.beforeAddHandler).to.have.been.calledWith(this.layoutView, 'foo'); - expect(this.onBeforeAddSpy).to.have.been.calledWith(this.layoutView, 'foo'); - }); - - it('should trigger a add:region event', function() { - expect(this.addHandler).to.have.been.calledWith(this.layoutView, 'foo', this.region); - expect(this.onAddSpy).to.have.been.calledWith(this.layoutView, 'foo', this.region); - }); }); describe('when adding a region to a layoutView, before it has been rendered', function() { @@ -164,29 +147,20 @@ describe('itemView - dynamic regions', function() { template: this.template, regions: { foo: '#foo' - }, - onBeforeRemoveRegion: function() {}, - onRemoveRegion: function() {} + } }); this.emptyHandler = this.sinon.spy(); - this.beforeRemoveHandler = this.sinon.spy(); - this.removeHandler = this.sinon.spy(); this.beforeDestroyHandler = this.sinon.spy(); this.destroyHandler = this.sinon.spy(); this.layoutView = new this.View(); - this.onBeforeRemoveSpy = this.sinon.spy(this.layoutView, 'onBeforeRemoveRegion'); - this.onRemoveSpy = this.sinon.spy(this.layoutView, 'onRemoveRegion'); - this.layoutView.render(); this.layoutView.getRegion('foo').show(new BBView()); this.region = this.layoutView.getRegion('foo'); this.region.on('empty', this.emptyHandler); - this.layoutView.on('before:remove:region', this.beforeRemoveHandler); - this.layoutView.on('remove:region', this.removeHandler); this.region.on('before:destroy', this.beforeDestroyHandler); this.region.on('destroy', this.destroyHandler); this.layoutView.removeRegion('foo'); @@ -204,16 +178,6 @@ describe('itemView - dynamic regions', function() { expect(this.destroyHandler).to.have.been.calledWith(this.region); }); - it('should trigger a before:remove:region event', function() { - expect(this.onBeforeRemoveSpy).to.have.been.calledWith(this.layoutView, 'foo'); - expect(this.beforeRemoveHandler).to.have.been.calledWith(this.layoutView, 'foo'); - }); - - it('should trigger a remove:region event', function() { - expect(this.onRemoveSpy).to.have.been.calledWith(this.layoutView, 'foo', this.region); - expect(this.removeHandler).to.have.been.calledWith(this.layoutView, 'foo', this.region); - }); - it('should remove the region', function() { expect(this.layoutView.getRegion('foo')).to.be.undefined; expect(this.layoutView.regions.foo).to.be.undefined;