Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CLEANUP] reducing private API surface of views. #13773

Merged
merged 1 commit into from
Jun 30, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions packages/ember-htmlbars/lib/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,9 +268,6 @@ Renderer.prototype.renderElementRemoval =
Renderer.prototype.willRemoveElement = function (/*view*/) {};

Renderer.prototype.willDestroyElement = function (view) {
if (view._willDestroyElement) {
view._willDestroyElement();
}
if (view.trigger) {
view.trigger('willDestroyElement');
view.trigger('willClearRender');
Expand All @@ -295,7 +292,7 @@ Renderer.prototype.didDestroyElement = function (view) {
if (view.trigger) {
view.trigger('didDestroyElement');
}
}; // Element destroyed so view.destroy shouldn't try to remove it removedFromDOM
};

export const InertRenderer = {
create({ dom }) {
Expand Down
1 change: 0 additions & 1 deletion packages/ember-views/lib/mixins/child_views_support.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ export default Mixin.create({

/**
Array of child views. You should never edit this array directly.
Instead, use `appendChild` and `removeFromParent`.

@property childViews
@type Array
Expand Down
63 changes: 6 additions & 57 deletions packages/ember-views/lib/mixins/view_support.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { assert, deprecate } from 'ember-metal/debug';
import { get } from 'ember-metal/property_get';
import run from 'ember-metal/run_loop';
import { guidFor } from 'ember-metal/utils';
import { Mixin } from 'ember-metal/mixin';
Expand Down Expand Up @@ -27,17 +26,18 @@ export default Mixin.create({
@param {Class,Mixin} klass Subclass of Ember.View (or Ember.View itself),
or an instance of Ember.Mixin.
@return Ember.View
@deprecated use `yield` and contextual components for composition instead.
@private
*/
nearestOfType(klass) {
let view = get(this, 'parentView');
let view = this.parentView;
let isOfType = klass instanceof Mixin ?
view => klass.detect(view) :
view => klass.detect(view.constructor);

while (view) {
if (isOfType(view)) { return view; }
view = get(view, 'parentView');
view = view.parentView;
}
},

Expand All @@ -47,14 +47,15 @@ export default Mixin.create({
@method nearestWithProperty
@param {String} property A property name
@return Ember.View
@deprecated use `yield` and contextual components for composition instead.
@private
*/
nearestWithProperty(property) {
let view = get(this, 'parentView');
let view = this.parentView;

while (view) {
if (property in view) { return view; }
view = get(view, 'parentView');
view = view.parentView;
}
},

Expand Down Expand Up @@ -110,21 +111,6 @@ export default Mixin.create({
return this._currentState.$(this, sel);
},

forEachChildView(callback) {
let childViews = this.childViews;

if (!childViews) { return this; }

let view, idx;

for (idx = 0; idx < childViews.length; idx++) {
view = childViews[idx];
callback(view);
}

return this;
},

/**
Appends the view's element to the specified parent element.

Expand Down Expand Up @@ -264,25 +250,6 @@ export default Mixin.create({
return this.appendTo(document.body);
},

/**
Removes the view's element from the element to which it is attached.

@method remove
@return {Ember.View} receiver
@private
*/
remove() {
// What we should really do here is wait until the end of the run loop
// to determine if the element has been re-appended to a different
// element.
// In the interim, we will just re-render if that happens. It is more
// important than elements get garbage collected.
if (!this.removedFromDOM) { this.destroyElement(); }

// Set flag to avoid future renders
this._willInsert = false;
},

/**
The HTML `id` of the view's element in the DOM. You can provide this
value yourself but it must be unique (just as in HTML):
Expand Down Expand Up @@ -557,24 +524,6 @@ export default Mixin.create({
}
},

/**
Removes the view from its `parentView`, if one is found. Otherwise
does nothing.

@method removeFromParent
@return {Ember.View} receiver
@private
*/
removeFromParent() {
let parent = this.parentView;

// Remove DOM element from parent
this.remove();

if (parent) { parent.removeChild(this); }
return this;
},

/**
You must call `destroy` on a view to destroy the view (and all of its
child views). This will remove the view from any parent node, then make
Expand Down
22 changes: 10 additions & 12 deletions packages/ember-views/lib/mixins/visibility_support.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,36 +67,34 @@ export default Mixin.create({

_notifyBecameVisible() {
this.trigger('becameVisible');

this.forEachChildView(view => {
let childViews = this.childViews;
for (let i = 0; i < childViews.length; i++) {
let view = childViews[i];
let isVisible = get(view, 'isVisible');

if (isVisible || isVisible === null) {
view._notifyBecameVisible();
}
});
}
},

_notifyBecameHidden() {
this.trigger('becameHidden');
this.forEachChildView(view => {
let childViews = this.childViews;
for (let i = 0; i < childViews.length; i++) {
let view = childViews[i];
let isVisible = get(view, 'isVisible');

if (isVisible || isVisible === null) {
view._notifyBecameHidden();
}
});
}
},

_isAncestorHidden() {
let parent = get(this, 'parentView');

let parent = this.parentView;
while (parent) {
if (get(parent, 'isVisible') === false) { return true; }

parent = get(parent, 'parentView');
parent = parent.parentView;
}

return false;
}
});
4 changes: 2 additions & 2 deletions packages/ember-views/tests/views/checkbox_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,12 @@ QUnit.test('checked property mirrors input value', function() {

equal(checkboxComponent.$().prop('checked'), true, 'changing the value property changes the DOM');

run(() => checkboxComponent.remove());
run(() => checkboxComponent.destroyElement());
run(() => checkboxComponent.append());

equal(checkboxComponent.$().prop('checked'), true, 'changing the value property changes the DOM');

run(() => checkboxComponent.remove());
run(() => checkboxComponent.destroyElement());
run(() => set(checkboxComponent, 'checked', false));
run(() => checkboxComponent.append());

Expand Down
2 changes: 1 addition & 1 deletion packages/ember-views/tests/views/view/append_to_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ QUnit.test('remove removes an element from the DOM', function() {

ok(jQuery('#' + get(view, 'elementId')).length === 1, 'precond - element was inserted');

run(() => view.remove());
run(() => view.destroyElement());

ok(jQuery('#' + get(view, 'elementId')).length === 0, 'remove removes an element from the DOM');
ok(EmberView.views[get(view, 'elementId')] === undefined, 'remove does not remove the view from the view hash');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ QUnit.skip('classNameBindings lifecycle test', function() {
equal(isWatching(view, 'priority'), true);

run(() => {
view.remove();
view.destroyElement();
view.set('priority', 'low');
});

Expand Down
8 changes: 4 additions & 4 deletions packages/ember-views/tests/views/view/is_visible_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ QUnit.test('should hide views when isVisible is false', function() {
run(() => set(view, 'isVisible', true));

ok(view.$().is(':visible'), 'the view is visible');
run(() => view.remove());
run(() => view.destroyElement());

deepEqual(warnings, [], 'no warnings were triggered');
});
Expand All @@ -57,15 +57,15 @@ QUnit.test('should hide element if isVisible is false before element is created'

ok(view.$().is(':hidden'), 'should be hidden');

run(() => view.remove());
run(() => view.destroyElement());

run(() => set(view, 'isVisible', true));

run(() => view.append());

ok(view.$().is(':visible'), 'view should be visible');

run(() => view.remove());
run(() => view.destroyElement());

deepEqual(warnings, [], 'no warnings were triggered');
});
Expand All @@ -84,7 +84,7 @@ QUnit.test('should hide views when isVisible is a CP returning false', function(
run(() => set(view, 'isVisible', true));

ok(view.$().is(':visible'), 'the view is visible');
run(() => view.remove());
run(() => view.destroyElement());

deepEqual(warnings, [], 'no warnings were triggered');
});
Expand Down
51 changes: 0 additions & 51 deletions packages/ember-views/tests/views/view/remove_test.js

This file was deleted.