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

Gauging interest in feedback and changes #1

Open
elskwid opened this issue Jun 17, 2015 · 0 comments
Open

Gauging interest in feedback and changes #1

elskwid opened this issue Jun 17, 2015 · 0 comments

Comments

@elskwid
Copy link

elskwid commented Jun 17, 2015

Hi there @rlivsey,

Started using this addon last week. Been a great help to get Sanitize wired up and working in our app. As I've been using it I found a few places where it's a little difficult to navigate and I was wondering if you'd be interested in (a) feedback and (b) changes in the form of pull requests.

For feedback (a):

  • testing: I would like to unit test my sanitizers but the need for a container makes it cumbersome in tests. I have a little fake container I inject but it's quite a bit of set up for a little test. Which brings me to -
  • sanitizers: I was a little surprised that it's the sanitizer config that is stored in the container rather than a sanitizer [Object] I could call and pass in the HTML. I did make the built-in sanitizer configs of [BASIC, RELAXED, and RESTRICTED] available using the container as well, which was nice!

Not coincidentally, the changes I would recommend would be related to these two items:

  • sanitizers: I think it would be neat to create an Ember Sanitizer Object that could get registered in the store and used throughout the codebase. Something like:
// sanitizer.js
export default Ember.Object.extend({
  config: null,

  sanitizeElement() {
    // ...
  },

  sanitizeHTML() {
    // ...
  }
});

// sanitizers/foo.js
import Sanitizer from 'ember-sanitize';

export default Sanitizer.extend({
  config: Sanitize.Config.BASIC;  
});

// somewhere else
this.container.lookup('sanitizer:foo').sanitizeHTML(myHTML);
  • testing: this change would mean that sanitizers could be tested without the container and would be self contained, it would be trivial to use this setup in the mixin as well.
  • blueprints: once you have objects like that you could create some blueprints to allow the creation of new serializers

I'll stop now. 😄 If you're interested in any of this I can put together a pull request or two and get moving.

Thanks for the project and for your time!

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

1 participant