Skip to content

Commit

Permalink
Merge pull request #19641 from nlfurniss/remove-is-visible
Browse files Browse the repository at this point in the history
Remove isVisible
  • Loading branch information
chancancode authored Jul 18, 2021
2 parents bba63f7 + 4942d48 commit 255a0dd
Show file tree
Hide file tree
Showing 6 changed files with 2 additions and 234 deletions.
13 changes: 0 additions & 13 deletions packages/@ember/-internals/glimmer/lib/component-managers/curly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { Factory, getOwner, Owner, setOwner } from '@ember/-internals/owner';
import { enumerableSymbol, guidFor, symbol } from '@ember/-internals/utils';
import { addChildView, setElementView, setViewElement } from '@ember/-internals/views';
import { assert, debugFreeze } from '@ember/debug';
import { EMBER_COMPONENT_IS_VISIBLE } from '@ember/deprecated-features';
import { _instrumentStart } from '@ember/instrumentation';
import { DEBUG } from '@glimmer/env';
import {
Expand Down Expand Up @@ -47,7 +46,6 @@ import {
createClassNameBindingRef,
createSimpleClassNameBindingRef,
installAttributeBinding,
installIsVisibleBinding,
parseAttributeBinding,
} from '../utils/bindings';

Expand Down Expand Up @@ -103,14 +101,6 @@ function applyAttributeBindings(
let id = component.elementId ? component.elementId : guidFor(component);
operations.setAttribute('id', createPrimitiveRef(id), false, null);
}

if (
EMBER_COMPONENT_IS_VISIBLE &&
installIsVisibleBinding !== undefined &&
seen.indexOf('style') === -1
) {
installIsVisibleBinding(rootRef, operations);
}
}

const EMPTY_POSITIONAL_ARGS: Reference[] = [];
Expand Down Expand Up @@ -390,9 +380,6 @@ export default class CurlyComponentManager
} else {
let id = component.elementId ? component.elementId : guidFor(component);
operations.setAttribute('id', createPrimitiveRef(id), false, null);
if (EMBER_COMPONENT_IS_VISIBLE) {
installIsVisibleBinding!(rootRef, operations);
}
}

if (classRef) {
Expand Down
10 changes: 0 additions & 10 deletions packages/@ember/-internals/glimmer/lib/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1091,16 +1091,6 @@ const Component = CoreView.extend(
@type String
@public
*/

/**
If `false`, the view will appear hidden in DOM.
@property isVisible
@type Boolean
@default null
@deprecated Use `<div hidden={{this.isHidden}}>` or `{{#if this.showComponent}} <MyComponent /> {{/if}}`
@public
*/
}
);

Expand Down
65 changes: 1 addition & 64 deletions packages/@ember/-internals/glimmer/lib/utils/bindings.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,16 @@
import { get } from '@ember/-internals/metal';
import { assert, deprecate } from '@ember/debug';
import { EMBER_COMPONENT_IS_VISIBLE } from '@ember/deprecated-features';
import { assert } from '@ember/debug';
import { dasherize } from '@ember/string';
import { DEBUG } from '@glimmer/env';
import { ElementOperations } from '@glimmer/interfaces';
import {
childRefFor,
childRefFromParts,
createComputeRef,
createPrimitiveRef,
Reference,
UNDEFINED_REFERENCE,
valueForRef,
} from '@glimmer/reference';
import { logTrackingStack } from '@glimmer/validator';
import { Component } from './curly-component-state-bucket';
import { htmlSafe, isHTMLSafe, SafeString } from './string';

function referenceForParts(rootRef: Reference<Component>, parts: string[]): Reference {
let isAttrs = parts[0] === 'attrs';
Expand Down Expand Up @@ -80,67 +75,9 @@ export function installAttributeBinding(
!(isSimple && isPath)
);

if (EMBER_COMPONENT_IS_VISIBLE && attribute === 'style' && createStyleBindingRef !== undefined) {
reference = createStyleBindingRef(reference, childRefFor(rootRef, 'isVisible'));
}

operations.setAttribute(attribute, reference, false, null);
}

const DISPLAY_NONE = 'display: none;';
const SAFE_DISPLAY_NONE = htmlSafe(DISPLAY_NONE);

let createStyleBindingRef:
| undefined
| ((inner: Reference, isVisible: Reference) => Reference<string | SafeString>);

export let installIsVisibleBinding:
| undefined
| ((rootRef: Reference<Component>, operations: ElementOperations) => void);

if (EMBER_COMPONENT_IS_VISIBLE) {
createStyleBindingRef = (inner: Reference, isVisibleRef: Reference) => {
return createComputeRef(() => {
let value = valueForRef(inner);
let isVisible = valueForRef(isVisibleRef);

if (DEBUG && isVisible !== undefined) {
deprecate(
`The \`isVisible\` property on classic component classes is deprecated. Was accessed:\n\n${logTrackingStack!()}`,
false,
{
id: 'ember-component.is-visible',
until: '4.0.0',
url: 'https://deprecations.emberjs.com/v3.x#toc_ember-component-is-visible',
for: 'ember-source',
since: {
enabled: '3.15.0-beta.1',
},
}
);
}

if (isVisible !== false) {
return value as string;
} else if (!value) {
return SAFE_DISPLAY_NONE;
} else {
let style = value + ' ' + DISPLAY_NONE;
return isHTMLSafe(value) ? htmlSafe(style) : style;
}
});
};

installIsVisibleBinding = (rootRef: Reference<Component>, operations: ElementOperations) => {
operations.setAttribute(
'style',
createStyleBindingRef!(UNDEFINED_REFERENCE, childRefFor(rootRef, 'isVisible')),
false,
null
);
};
}

export function createClassNameBindingRef(
rootRef: Reference<Component>,
microsyntax: string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
classes,
equalTokens,
equalsElement,
styles,
runTask,
runLoopSettled,
} from 'internal-test-helpers';
Expand All @@ -18,7 +17,7 @@ import { Object as EmberObject, A as emberA } from '@ember/-internals/runtime';
import { jQueryDisabled } from '@ember/-internals/views';

import { Component, compile, htmlSafe } from '../../utils/helpers';
import { backtrackingMessageFor, debugStackMessageFor } from '../../utils/debug-stack';
import { backtrackingMessageFor } from '../../utils/debug-stack';

moduleFor(
'Components test: curly components',
Expand Down Expand Up @@ -3150,149 +3149,6 @@ moduleFor(
}, /You must call `super.init\(...arguments\);` or `this._super\(...arguments\)` when overriding `init` on a framework object. Please update .*/);
}

['@test should toggle visibility with isVisible'](assert) {
let assertStyle = (expected) => {
let matcher = styles(expected);
let actual = this.firstChild.getAttribute('style');

assert.pushResult({
result: matcher.match(actual),
message: matcher.message(),
actual,
expected,
});
};

this.registerComponent('foo-bar', {
template: `<p>foo</p>`,
});

expectDeprecation(() => {
this.render(`{{foo-bar id="foo-bar" isVisible=this.visible}}`, {
visible: false,
});
}, debugStackMessageFor('The `isVisible` property on classic component classes is deprecated. Was accessed:', { renderTree: ['foo-bar'] }));

assertStyle('display: none;');

this.assertStableRerender();

expectDeprecation(() => {
runTask(() => {
set(this.context, 'visible', true);
});
}, debugStackMessageFor('The `isVisible` property on classic component classes is deprecated. Was accessed:', { renderTree: ['foo-bar'] }));

assertStyle('');

expectDeprecation(() => {
runTask(() => {
set(this.context, 'visible', false);
});
}, debugStackMessageFor('The `isVisible` property on classic component classes is deprecated. Was accessed:', { renderTree: ['foo-bar'] }));

assertStyle('display: none;');
}

['@test isVisible does not overwrite component style']() {
this.registerComponent('foo-bar', {
ComponentClass: Component.extend({
attributeBindings: ['style'],
style: htmlSafe('color: blue;'),
}),

template: `<p>foo</p>`,
});

expectDeprecation(() => {
this.render(`{{foo-bar id="foo-bar" isVisible=this.visible}}`, {
visible: false,
});
}, debugStackMessageFor('The `isVisible` property on classic component classes is deprecated. Was accessed:', { renderTree: ['foo-bar'] }));

this.assertComponentElement(this.firstChild, {
tagName: 'div',
attrs: { id: 'foo-bar', style: styles('color: blue; display: none;') },
});

this.assertStableRerender();

expectDeprecation(() => {
runTask(() => {
set(this.context, 'visible', true);
});
}, debugStackMessageFor('The `isVisible` property on classic component classes is deprecated. Was accessed:', { renderTree: ['foo-bar'] }));

this.assertComponentElement(this.firstChild, {
tagName: 'div',
attrs: { id: 'foo-bar', style: styles('color: blue;') },
});

expectDeprecation(() => {
runTask(() => {
set(this.context, 'visible', false);
});
}, debugStackMessageFor('The `isVisible` property on classic component classes is deprecated. Was accessed:', { renderTree: ['foo-bar'] }));

this.assertComponentElement(this.firstChild, {
tagName: 'div',
attrs: { id: 'foo-bar', style: styles('color: blue; display: none;') },
});
}

['@test adds isVisible binding when style binding is missing and other bindings exist'](
assert
) {
let assertStyle = (expected) => {
let matcher = styles(expected);
let actual = this.firstChild.getAttribute('style');

assert.pushResult({
result: matcher.match(actual),
message: matcher.message(),
actual,
expected,
});
};

this.registerComponent('foo-bar', {
ComponentClass: Component.extend({
attributeBindings: ['foo'],
foo: 'bar',
}),
template: `<p>foo</p>`,
});

expectDeprecation(() => {
this.render(`{{foo-bar id="foo-bar" foo=this.foo isVisible=this.visible}}`, {
visible: false,
foo: 'baz',
});
}, debugStackMessageFor('The `isVisible` property on classic component classes is deprecated. Was accessed:', { renderTree: ['foo-bar'] }));

assertStyle('display: none;');

this.assertStableRerender();

expectDeprecation(() => {
runTask(() => {
set(this.context, 'visible', true);
});
}, debugStackMessageFor('The `isVisible` property on classic component classes is deprecated. Was accessed:', { renderTree: ['foo-bar'] }));

assertStyle('');

expectDeprecation(() => {
runTask(() => {
set(this.context, 'visible', false);
set(this.context, 'foo', 'woo');
});
}, debugStackMessageFor('The `isVisible` property on classic component classes is deprecated. Was accessed:', { renderTree: ['foo-bar'] }));

assertStyle('display: none;');
assert.equal(this.firstChild.getAttribute('foo'), 'woo');
}

['@test it can use readDOMAttr to read input value']() {
let component;
let assertElement = (expectedValue) => {
Expand Down
1 change: 0 additions & 1 deletion packages/@ember/deprecated-features/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,5 @@ export const ALIAS_METHOD = !!'3.9.0';
export const APP_CTRL_ROUTER_PROPS = !!'3.10.0-beta.1';
export const FUNCTION_PROTOTYPE_EXTENSIONS = !!'3.11.0-beta.1';
export const MOUSE_ENTER_LEAVE_MOVE_EVENTS = !!'3.13.0-beta.1';
export const EMBER_COMPONENT_IS_VISIBLE = !!'3.15.0-beta.1';
export const PARTIALS = !!'3.15.0-beta.1';
export const GLOBALS_RESOLVER = !!'3.16.0-beta.1';
1 change: 0 additions & 1 deletion tests/docs/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,6 @@ module.exports = {
'isPresent',
'isRejected',
'isSettled',
'isVisible',
'join',
'jQuery',
'keyDown',
Expand Down

0 comments on commit 255a0dd

Please sign in to comment.