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

Strip test selectors defined in component javascript #10

Closed
blimmer opened this issue Mar 8, 2016 · 8 comments
Closed

Strip test selectors defined in component javascript #10

blimmer opened this issue Mar 8, 2016 · 8 comments

Comments

@blimmer
Copy link
Contributor

blimmer commented Mar 8, 2016

This is a great idea! However, many times I find myself doing something like this:

export default Ember.Component.extend({
  attributeBindings: ['data-test-id'],
  'data-test-id': Ember.computed('model.id', function() {
     return `model-name-${id}-component`;
  }),
});

Just browsing the code, it looks like this wouldn't be stripped (since it's using a handlebars AST parser).

@marcoow
Copy link
Member

marcoow commented Mar 8, 2016

We find it more convenient to have the data attributes in the templates as that also allows more flexibility since you can have data attributes on individual elements as well as opposed to only the component itself. Of course it would be nice to support this case as well though.

Would be great if you could submit a PR implementing that although I'm not sure how this would best be done.

@marcoow
Copy link
Member

marcoow commented Mar 21, 2016

It would probably also make sense to always (except for in production of course) add a data-test-<component-name> attribute to the component so you can always select the component itself, e.g. data-test-blog-post.

@marcoow
Copy link
Member

marcoow commented Mar 21, 2016

I'm using this in a project now:

// app/mixins/components/has-test-selector.js
import Ember from 'ember';
import ENV from '../../config/environment';

const { computed } = Ember;
const { getPrototypeOf } = Object;

let TestSelectorMixin;

if (ENV.environment !== 'production') {
  TestSelectorMixin = Ember.Mixin.create({
    attributeBindings: 'data-test-selector',

    'data-test-selector': computed(function() {
      return getPrototypeOf(this)._debugContainerKey.replace(/^component:/, '');
    })
  });
} else {
  TestSelectorMixin = Ember.Mixin.create();
}

export default TestSelectorMixin;

I guess sth. like this could be added to ember-test-selectors as well.

Of course this.__proto__._debugContainerKey is private Ember.js API but there seems to be consensus that it could be publicized so it shouldn't be too bad to rely on it now.

@chrisdpeters
Copy link

@marcoow I like this idea.

Would it make more sense if it were something a tad more descriptive like component, rather than selector because the selector is being based on the component name? Just an idea/question; I really don't know if that's any better.

@marcoow
Copy link
Member

marcoow commented Sep 5, 2016

@chrisdpeters: yeah, sounds like a good idea.

@bcardarella
Copy link

@marcoow any update on this?

@marcoow
Copy link
Member

marcoow commented Oct 26, 2016

I'm hoping someone will find some time to do this soon. It would definitely be a great addition. Someone on our team might actually be able to pick this up in the next weeks but we've all been pretty busy lately.

@marcoow
Copy link
Member

marcoow commented Jan 11, 2017

This has been fixed via #45

@marcoow marcoow closed this as completed Jan 11, 2017
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

No branches or pull requests

4 participants