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

[Glimmer] Port component elementid test. #13208

Merged
merged 1 commit into from
Apr 5, 2016
Merged

[Glimmer] Port component elementid test. #13208

merged 1 commit into from
Apr 5, 2016

Conversation

jheth
Copy link
Contributor

@jheth jheth commented Mar 28, 2016

Moved over the one elementid test and added another one for custom elementid.

Issue #13127

this.render('{{foo-bar id="custom-id"}}');

var foundId = this.$('h1').attr('id');
assert.equal(foundId, 'custom-id');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a new failing test for glimmer2. It still generates an emberXX id.

@rwjblue
Copy link
Member

rwjblue commented Mar 28, 2016

Looks great!

@chancancode - r?

}
});

QUnit.test('passing undefined elementId results in a default elementId', function() {
Copy link
Member

Choose a reason for hiding this comment

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

Ported to @test passing undefined elementId results in a default elementId

@homu
Copy link
Contributor

homu commented Apr 3, 2016

☔ The latest upstream changes (presumably #13236) made this pull request unmergeable. Please resolve the merge conflicts.

@rwjblue
Copy link
Member

rwjblue commented Apr 3, 2016

@jheth - this looks great, I think merging #13236 caused some rebase issues here (and added at least one elementId test). Can you double check and rebase?

@jheth
Copy link
Contributor Author

jheth commented Apr 4, 2016

@rwjblue I've rebased and pushed. I moved my tests up in the file to be next to the other elementid test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants