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

Handle components with empty tag name #323

Merged
merged 3 commits into from
Mar 15, 2019
Merged

Conversation

ssutar
Copy link

@ssutar ssutar commented Feb 21, 2019

This is an extension to existing PR #291 which handles addition of attribute supportsDataTestProperties

I tried using the PR but it throws an exception You cannot use attributeBindings on a tag-less component while running following code:

component.set('attributeBindings', attributeBindings);

as the attribute bindings can not be set on component with empty tagName

This PR just adds an early exit in util bind-data-test-attributes in case of tag-less component and an acceptance test on top of existing PR #291

Resolves #290, Closes #291

@rwjblue
Copy link
Contributor

rwjblue commented Feb 22, 2019

@ssutar - It may be easier to target @bendemboski's fork + branch with your additional commit. That way we can keep the conversation on the overall fix over in #291...

@Turbo87
Copy link
Collaborator

Turbo87 commented Feb 22, 2019

@rwjblue I think it's fine this way. Makes it easier for me to merge directly once I find time to do a proper review.

@Turbo87 Turbo87 changed the base branch from master to v2.x March 15, 2019 14:43
Copy link
Collaborator

@Turbo87 Turbo87 left a comment

Choose a reason for hiding this comment

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

this looks good, thanks a lot @ssutar and @bendemboski!

@Turbo87 Turbo87 merged commit 663d48a into mainmatter:v2.x Mar 15, 2019
@Turbo87
Copy link
Collaborator

Turbo87 commented Mar 15, 2019

released as v2.1.0 🎉

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

Successfully merging this pull request may close these issues.

3 participants