-
Notifications
You must be signed in to change notification settings - Fork 12
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
What about making ampersand-class-extend
an amp
module?
#66
Comments
@HenrikJoreteg what do you think about this proposal? |
Yeah, seems logical as long as we can name things in a way that make sense. I'm hesitant to re-name but having |
Just a suggestion to keep the nomenclature matching the other 'class' methods (i.e. amp-add-class) which would make this I'm a little lukewarm about renaming amp-extend since it's an already established programming concept (extending objects) versus this other subset of methods (that manipulate class attributes on a DOM element) |
Any name with "class" in it is confusing to me given that all of the existing ones deal with DOM classes. How about following the lead of Closure and Node and going with |
Also worth thinking about is what happens to a utility like this as we migrate to ES6 classes? |
I like the re: ES6, I am not sure. I guess this takes some time. |
It's a utility module as well. Would be a clean and consistent approach to merge it into
amp
instead of having it as an individual module. What do you think?It requires a small change to the objects
extend
module already inamp
but that could be a simple name change, for example:amp-object-extend
amp-class-extend
The text was updated successfully, but these errors were encountered: