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

[Glimmer2] inline if #12920

Merged
merged 1 commit into from
Feb 24, 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
4 changes: 3 additions & 1 deletion packages/ember-glimmer/lib/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,11 @@ class PropertyReference {
}

import { default as concat } from './helpers/concat';
import { default as inlineIf } from './helpers/inline-if';

const helpers = {
concat
concat,
if: inlineIf
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have an inlineUnless helper in Ember too 😄

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can quickly follow this PR up with one adding support for inline unless
if that's OK?
On 17 Feb 2016 6:47 p.m., "Godfrey Chan" notifications@github.com wrote:

In packages/ember-glimmer/lib/environment.js
#12920 (comment):

const helpers = {

  • concat
  • concat,
  • if: inlineIf

We have an inlineUnless helper in Ember too [image: 😄]


Reply to this email directly or view it on GitHub
https://github.com/emberjs/ember.js/pull/12920/files#r53210816.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

};

class EmberConditionalReference extends ConditionalReference {
Expand Down
39 changes: 39 additions & 0 deletions packages/ember-glimmer/lib/helpers/inline-if.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
@module ember
@submodule ember-templates
*/

import { toBool as emberToBool } from './if-unless';
import { assert } from 'ember-metal/debug';

/**
The inline `if` helper conditionally renders a single property or string.
This helper acts like a ternary operator. If the first property is truthy,
the second argument will be displayed, if not, the third argument will be
displayed
```handlebars
{{if useLongGreeting "Hello" "Hi"}} Alex
```
You can use the `if` helper inside another helper as a subexpression.
```handlebars
{{some-component height=(if isBig "100" "10")}}
```
@method if
@for Ember.Templates.helpers
@public
*/
export default function inlineIf(args) {
assert(
'The inline form of the `if` and `unless` helpers expect two or ' +
'three arguments, e.g. `{{if trialExpired \'Expired\' expiryDate}}` ',
args.length === 2 || args.length === 3
);

if (emberToBool(args[0])) {
return args[1];
} else {
//TODO: always return `args[2]` post glimmer2: https://github.com/emberjs/ember.js/pull/12920#discussion_r53213383
let falsyArgument = args[2];
return falsyArgument === undefined ? '' : falsyArgument;
}
}
84 changes: 84 additions & 0 deletions packages/ember-glimmer/tests/integration/helpers/inline-if-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { moduleFor } from '../../utils/test-case';
import { set } from 'ember-metal/property_set';

import {
BASIC_TRUTHY_TESTS,
BASIC_FALSY_TESTS,
SharedHelperConditionalsTest
} from '../../utils/shared-conditional-tests';

moduleFor('Helpers test: inline {{if}}', class extends SharedHelperConditionalsTest {

templateFor({ cond, truthy, falsy }) {
return `{{if ${cond} ${truthy} ${falsy}}}`;
}

['@test it can omit the falsy argument']() {
this.render(`{{if cond1 'T1'}}{{if cond2 'T2'}}`, { cond1: true, cond2: false });

this.assertText('T1');

this.runTask(() => this.rerender());

this.assertText('T1');

this.runTask(() => set(this.context, 'cond1', false));

this.assertText('');

this.runTask(() => {
set(this.context, 'cond1', true);
set(this.context, 'cond2', true);
});

this.assertText('T1T2');

this.runTask(() => {
set(this.context, 'cond1', true);
set(this.context, 'cond2', false);
});

this.assertText('T1');
}

['@test it raises when there are more than three arguments']() {
expectAssertion(() => {
this.render(`{{if condition 'a' 'b' 'c'}}`, { condition: true });
}, /The inline form of the `if` and `unless` helpers expect two or three arguments/);
}

['@test it raises when there are less than two arguments']() {
expectAssertion(() => {
this.render(`{{if condition}}`, { condition: true });
}, /The inline form of the `if` and `unless` helpers expect two or three arguments/);
}

}, BASIC_TRUTHY_TESTS, BASIC_FALSY_TESTS);

moduleFor('@glimmer Helpers test: {{if}} used with another helper', class extends SharedHelperConditionalsTest {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@glimmer only as several of these tests fail in @htmlbars (see #12925)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😢


wrapperFor(templates) {
return `{{concat ${templates.join(' ')}}}`;
}

templateFor({ cond, truthy, falsy }) {
return `(if ${cond} ${truthy} ${falsy})`;
}

});

moduleFor('@glimmer Helpers test: {{if}} used in attribute position', class extends SharedHelperConditionalsTest {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@glimmer only as several of these tests fail in @htmlbars

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😢


wrapperFor(templates) {
return `<div data-foo="${templates.join('')}" />`;
}

templateFor({ cond, truthy, falsy }) {
return `{{if ${cond} ${truthy} ${falsy}}}`;
}

textValue() {
return this.$('div', this.element).attr('data-foo');
}

});
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ import { set } from 'ember-metal/property_set';
import {
BASIC_TRUTHY_TESTS,
BASIC_FALSY_TESTS,
SharedConditionalsTest
SharedSyntaxConditionalsTest
} from '../../utils/shared-conditional-tests';

moduleFor('Syntax test: {{#if}}', class extends SharedConditionalsTest {
moduleFor('Syntax test: {{#if}}', class extends SharedSyntaxConditionalsTest {

templateFor({ cond, truthy, falsy }) {
return `{{#if ${cond}}}${truthy}{{else}}${falsy}{{/if}}`;
Expand Down Expand Up @@ -42,7 +42,7 @@ moduleFor('Syntax test: {{#if}}', class extends SharedConditionalsTest {

}, BASIC_TRUTHY_TESTS, BASIC_FALSY_TESTS);

moduleFor('Syntax test: {{#unless}}', class extends SharedConditionalsTest {
moduleFor('Syntax test: {{#unless}}', class extends SharedSyntaxConditionalsTest {

templateFor({ cond, truthy, falsy }) {
return `{{#unless ${cond}}}${falsy}{{else}}${truthy}{{/unless}}`;
Expand Down
6 changes: 3 additions & 3 deletions packages/ember-glimmer/tests/integration/syntax/with-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,18 @@ import { set } from 'ember-metal/property_set';
import {
BASIC_TRUTHY_TESTS,
BASIC_FALSY_TESTS,
SharedConditionalsTest
SharedSyntaxConditionalsTest
} from '../../utils/shared-conditional-tests';
import { RenderingTest } from '../../utils/test-case';

moduleFor('Syntax test: {{#with}}', class extends SharedConditionalsTest {
moduleFor('Syntax test: {{#with}}', class extends SharedSyntaxConditionalsTest {
templateFor({ cond, truthy, falsy }) {
return `{{#with ${cond}}}${truthy}{{else}}${falsy}{{/with}}`;
}

}, BASIC_TRUTHY_TESTS, BASIC_FALSY_TESTS);

moduleFor('Syntax test: {{#with as}}', class extends SharedConditionalsTest{
moduleFor('Syntax test: {{#with as}}', class extends SharedSyntaxConditionalsTest {
templateFor({ cond, truthy, falsy }) {
return `{{#with ${cond} as |test|}}${truthy}{{else}}${falsy}{{/with}}`;
}
Expand Down
17 changes: 15 additions & 2 deletions packages/ember-glimmer/tests/utils/abstract-test-case.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ const packageTag = `@${packageName} `;
export function moduleFor(description, TestClass, ...generators) {
let context;

QUnit.module(`[${packageName}] ${description}`, {
let modulePackagePrefixMatch = description.match(/^@(\w*)/); //eg '@glimmer' or '@htmlbars'
let modulePackagePrefix = modulePackagePrefixMatch ? modulePackagePrefixMatch[1] : '';
let descriptionWithoutPackagePrefix = description.replace(/^@\w* /, '');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this adds support for module prefixes such as @glimmer and @htmlbars and is needed as the new comprehensive shared tests have highlighted some issues with htmlbars


QUnit.module(`[${packageName}] ${descriptionWithoutPackagePrefix}`, {
setup() {
context = new TestClass();
},
Expand All @@ -39,6 +43,10 @@ export function moduleFor(description, TestClass, ...generators) {
}

function generateTest(name) {
if (modulePackagePrefix && packageName !== modulePackagePrefix) {
return;
}

if (name.indexOf('@test ') === 0) {
QUnit.test(name.slice(5), assert => context[name](assert));
} else if (name.indexOf('@skip ') === 0) {
Expand Down Expand Up @@ -78,6 +86,7 @@ export class RenderingTest extends TestCase {
let owner = this.owner = buildOwner();
let env = this.env = new Environment({ dom, owner });
this.renderer = new Renderer(dom, { destinedForDOM: true, env });
this.$ = jQuery;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, this is not what I meant, I mean something like how this.$() works in components, but I can fix it

this.element = jQuery('#qunit-fixture')[0];
this.component = null;
this.snapshot = null;
Expand Down Expand Up @@ -169,8 +178,12 @@ export class RenderingTest extends TestCase {
}
}

textValue() {
return jQuery(this.element).text();
}

assertText(text) {
assert.strictEqual(jQuery(this.element).text(), text, '#qunit-fixture content');
assert.strictEqual(this.textValue(), text, '#qunit-fixture content');
}

assertHTML(html) {
Expand Down
Loading