Skip to content

Commit

Permalink
[CLEANUP] Unify glimmer behavior with htmlbars and remove unsed code
Browse files Browse the repository at this point in the history
Normalize attribute assertions and behavior across htmlbars and glimmer

Remove _defaultTagName which was for support of legacy each AST plugin
which was removed

Some general cleanup
  • Loading branch information
krisselden committed Jul 26, 2016
1 parent 8d701b8 commit e447dd8
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ moduleFor('Components test: fragment components', class extends RenderingTest {
}, /You cannot use `classNameBindings` on a tag-less component/);
}

['@glimmer throws an error if `tagName` is an empty string and `attributeBindings` are specified']() {
['@test throws an error if `tagName` is an empty string and `attributeBindings` are specified']() {
let template = `hit dem folks`;
let FooBarComponent = Component.extend({
tagName: '',
Expand All @@ -117,7 +117,7 @@ moduleFor('Components test: fragment components', class extends RenderingTest {
}, /You cannot use `attributeBindings` on a tag-less component/);
}

['@glimmer throws an error if `tagName` is an empty string and `elementId` is specified via JS']() {
['@test throws an error if `tagName` is an empty string and `elementId` is specified via JS']() {
let template = `hit dem folks`;
let FooBarComponent = Component.extend({
tagName: '',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import getCellOrValue from 'ember-htmlbars/hooks/get-cell-or-value';
import { get } from 'ember-metal/property_get';
import { MUTABLE_CELL } from 'ember-views/compat/attrs-proxy';
import { instrument } from 'ember-htmlbars/system/instrumentation-support';
import LegacyEmberComponent, { HAS_BLOCK } from 'ember-htmlbars/component';
import EmberComponent, { HAS_BLOCK } from 'ember-htmlbars/component';
import extractPositionalParams from 'ember-htmlbars/utils/extract-positional-params';
import { setOwner, getOwner } from 'container/owner';

Expand Down Expand Up @@ -33,7 +33,7 @@ ComponentNodeManager.create = function ComponentNodeManager_create(renderNode, e
layout,
templates } = options;

component = component || LegacyEmberComponent;
component = component || EmberComponent;

let createOptions = {
parentView,
Expand Down Expand Up @@ -74,7 +74,6 @@ ComponentNodeManager.create = function ComponentNodeManager_create(renderNode, e
return new ComponentNodeManager(component, parentScope, renderNode, attrs, results.block, results.createdElement);
};


function configureTagName(attrs, tagName, component, createOptions) {
if (attrs.tagName) {
createOptions.tagName = getValue(attrs.tagName);
Expand All @@ -86,7 +85,6 @@ function configureCreateOptions(attrs, createOptions) {
// instance. Make sure we use getValue() to get them from `attrs` since
// they are still streams.
if (attrs.id) { createOptions.elementId = getValue(attrs.id); }
if (attrs._defaultTagName) { createOptions._defaultTagName = getValue(attrs._defaultTagName); }
}

ComponentNodeManager.prototype.render = function ComponentNodeManager_render(_env, visitor) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ ViewNodeManager.create = function ViewNodeManager_create(renderNode, env, attrs,

if (attrs && attrs.id) { options.elementId = getValue(attrs.id); }
if (attrs && attrs.tagName) { options.tagName = getValue(attrs.tagName); }
if (attrs && attrs._defaultTagName) { options._defaultTagName = getValue(attrs._defaultTagName); }

component = componentInfo.component = createOrUpdateComponent(found.component, options, found.createOptions, renderNode, env, attrs);

Expand Down
45 changes: 25 additions & 20 deletions packages/ember-htmlbars/lib/system/build-component-template.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,22 @@ export default function buildComponentTemplate({ component, tagName, layout, out
elementTemplate.meta = meta;

blockToRender = createElementBlock(elementTemplate, blockToRender, component);
} else {
validateTaglessComponent(component);
}

assert('You cannot use `classNameBindings` on a tag-less component: ' + component.toString(), (() => {
let { classNameBindings } = component;
return tagName !== '' || !classNameBindings || classNameBindings.length === 0;
})());

assert('You cannot use `elementId` on a tag-less component: ' + component.toString(), (() => {
let { elementId } = component;
return tagName !== '' || (!elementId && elementId !== '');
})());

assert('You cannot use `attributeBindings` on a tag-less component: ' + component.toString(), (() => {
let { attributeBindings } = component;
return tagName !== '' || !attributeBindings || attributeBindings.length === 0;
})());
}

// tagName is one of:
Expand Down Expand Up @@ -124,7 +137,7 @@ function tagNameFor(view) {
let tagName = view.tagName;

if (tagName === null || tagName === undefined) {
tagName = view._defaultTagName || 'div';
tagName = 'div';
}

return tagName;
Expand All @@ -135,7 +148,6 @@ function tagNameFor(view) {
function normalizeComponentAttributes(component, attrs) {
let normalized = {};
let attributeBindings = component.attributeBindings;
let streamBasePath = component.isComponent ? '' : 'view.';

if (attrs.id && getValue(attrs.id)) {
// Do not allow binding to the `id`
Expand All @@ -154,7 +166,7 @@ function normalizeComponentAttributes(component, attrs) {
if (colonIndex !== -1) {
let attrProperty = attr.substring(0, colonIndex);
attrName = attr.substring(colonIndex + 1);
expression = buildStatement('get', `${streamBasePath}${attrProperty}`);
expression = buildStatement('get', attrProperty);
} else if (attrs[attr]) {
// TODO: For compatibility with 1.x, we probably need to `set`
// the component's attribute here if it is a CP, but we also
Expand All @@ -164,19 +176,21 @@ function normalizeComponentAttributes(component, attrs) {
expression = buildStatement('value', attrs[attr]);
} else {
attrName = attr;
expression = buildStatement('get', `${streamBasePath}${attr}`);
expression = buildStatement('get', attr);
}

assert('You cannot use class as an attributeBinding, use classNameBindings instead.', attrName !== 'class');
normalized[attrName] = expression;
}
}

normalized.role = buildStatement('get', 'ariaRole');

if (attrs.tagName) {
component.tagName = attrs.tagName;
}

let normalizedClass = normalizeClass(component, attrs, streamBasePath);
let normalizedClass = normalizeClass(component, attrs);
if (normalizedClass) {
normalized.class = normalizedClass;
}
Expand All @@ -195,7 +209,7 @@ function normalizeComponentAttributes(component, attrs) {
return normalized;
}

function normalizeClass(component, attrs, streamBasePath) {
function normalizeClass(component, attrs) {
let normalizedClass = [];
let classNames = get(component, 'classNames');
let classNameBindings = get(component, 'classNameBindings');
Expand All @@ -209,7 +223,7 @@ function normalizeClass(component, attrs, streamBasePath) {
}

if (attrs.classBinding) {
normalizeClasses(attrs.classBinding.split(' '), normalizedClass, streamBasePath);
normalizeClasses(attrs.classBinding.split(' '), normalizedClass);
}

if (classNames) {
Expand All @@ -219,7 +233,7 @@ function normalizeClass(component, attrs, streamBasePath) {
}

if (classNameBindings) {
normalizeClasses(classNameBindings, normalizedClass, streamBasePath);
normalizeClasses(classNameBindings, normalizedClass);
}

if (normalizeClass.length) {
Expand All @@ -240,23 +254,14 @@ function normalizeClasses(classes, output, streamBasePath) {
continue;
}

let prop = `${streamBasePath}${propName}`;

output.push(buildStatement('subexpr', '-normalize-class', [
// params
buildStatement('value', propName),
buildStatement('get', prop)
buildStatement('get', propName)
], [
// hash
'activeClass', activeClass,
'inactiveClass', inactiveClass
]));
}
}

function validateTaglessComponent(component) {
assert('You cannot use `classNameBindings` on a tag-less component: ' + component.toString(), (() => {
let classNameBindings = component.classNameBindings;
return !classNameBindings || classNameBindings.length === 0;
})());
}
8 changes: 0 additions & 8 deletions packages/ember-views/lib/mixins/view_support.js
Original file line number Diff line number Diff line change
Expand Up @@ -428,14 +428,6 @@ export default Mixin.create({
// the default case and a user-specified tag.
tagName: null,

/*
Used to specify a default tagName that can be overridden when extending
or invoking from a template.
@property _defaultTagName
@private
*/

// .......................................................
// CORE DISPLAY METHODS
//
Expand Down
19 changes: 1 addition & 18 deletions packages/ember-views/lib/views/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -512,24 +512,7 @@ var View = CoreView.extend(
VisibilitySupport,
CompatAttrsProxy,
AriaRoleSupport,
ViewMixin, {
attributeBindings: ['ariaRole:role'],

/**
Given a property name, returns a dasherized version of that
property name if the property evaluates to a non-falsy value.
For example, if the view has property `isUrgent` that evaluates to true,
passing `isUrgent` to this method will return `"is-urgent"`.
@method _classStringForProperty
@param property
@private
*/
_classStringForProperty(parsedPath) {
return View._classStringForValue(parsedPath.path, parsedPath.stream.value(), parsedPath.className, parsedPath.falsyClassName);
}
});
ViewMixin);

// jscs:enable validateIndentation

Expand Down

0 comments on commit e447dd8

Please sign in to comment.