-
Notifications
You must be signed in to change notification settings - Fork 16
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
support a transform callback for complex prefixing #16
Conversation
@spencer516 Can you give some details on how this feature is supposed to be used? I'm OK with exposing the transform function for custom use cases but I'd like to know what they are. |
Sure — the simplest use case is that I'm using Foundation, which adds styles directly to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just two minor changes needed and I'll be able to merge this PR, thanks!
prefix: '.hello', | ||
transform: function (prefix, selector, prefixAndSelector) { | ||
if (selector === 'body') { | ||
return 'body.body-picker' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use the prefix
param here instead of hard coding a custom one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what you mean here...can you clarify?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's minor thing but I'd rather do return 'body' + prefix
here, just so we're checking that the prefix
is being passed correctly to this method.
exclude: ['.c'], | ||
|
||
// Optional transform callback for case-by-case overrides | ||
transform: function (prefix, selector, prefixAndSelector) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be awesome if you could add an extra paragraph at the end under Options
about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do!
775dd45
to
20b3078
Compare
I made the changes...have a look and let me know if all is A-OK. Cheers! |
@spencer516 Looks good, I'll publish the changes in the next release. Thanks! |
This has been working great for my use case, however, I've got some situations where I don't want to exclude a selector, but I also don't want to prefix it.
So, this supports an added
transform
callback, so the consumer can manually override in specific cases. Let me know if there's anything you'd like changed — would love to get this merged!Thanks!