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

Updated and simplified README file #48

Merged
merged 8 commits into from
Jan 12, 2017
Merged

Conversation

Turbo87
Copy link
Collaborator

@Turbo87 Turbo87 commented Jan 12, 2017

Copy link
Member

@marcoow marcoow left a comment

Choose a reason for hiding this comment

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

looks good - just a few small change requests

@@ -82,65 +51,47 @@ allows to encode more data in the markup like e.g. the post's id:
</article>
```
Copy link
Member

Choose a reason for hiding this comment

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

Would be cool to add an example for a component with some data-test-* attributes, e.g.:

{{comments-list data-test-comments-for=post.id}}

or so

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

</article>
```
- Automatically binds properties starting with `data-test-` on all components
for development/testing builds
Copy link
Member

Choose a reason for hiding this comment

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

I guess we can leave the "for development/testing builds" as it might confuse people. Basically we're always doing that (regardless of the env) and strip these assignments when strip is true.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that's not quite true, the initializer is only included when we're not stripping. in other words: in production mode there is not initializer and no automatic binding.

Copy link
Member

Choose a reason for hiding this comment

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

Right but the point is that this doesn't actually depend on the environment but on the strip setting (which happens to default on the env).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

then we probably shouldn't mention "for production builds" above either?

Copy link
Member

Choose a reason for hiding this comment

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

Hm, good point - let's maybe just leave it as it is ;)

`ember-test-selectors` makes sure to remove all these `data-test-*` attributes in the
`production` environment so that __users will have perfectly clean HTML
delivered__:
Once you've done that you can use our `testSelector()` function to create
Copy link
Contributor

Choose a reason for hiding this comment

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

Should our be the here?

Copy link
Contributor

@pangratz pangratz left a comment

Choose a reason for hiding this comment

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

LGTM!

@Turbo87 Turbo87 merged commit 07987bf into mainmatter:master Jan 12, 2017
@Turbo87 Turbo87 deleted the readme branch January 12, 2017 12:54
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