-
Notifications
You must be signed in to change notification settings - Fork 446
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
Adds extraclasses option to events; #281
Conversation
Replace element instead of putting stuff inside it. (keep extraClasses) Correct bug when lengthOfTime is not a month;
} | ||
|
||
} | ||
} ) |
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.
This looks pretty verbose for what I think is just a space delineated concatenation of the classes.
What if it was just:
extraClasses += (" " + this.options.classes.event)
+ (_.isArray(event.extraClasses) ? " " + event.extraClasses.join(" ") : "");
?
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.
Seems super legit :)
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 tried to preserve initial codestyle.
Thanks @long-lazuli I'm going to get to these today. Also (FYI) you don't have to update the version, that all happens during a compilation step in master. This one was the only open PR but if we had 2 or more that all incremented the version they'd eventually conflict :P I can reset it during the merge. |
Thx for this explanation about versions, @mikegioia |
* Fixes audit warnings and errors with new packages; adding package lock file * Audited NPM packages and linted src file. All lint errors and warnings have been fixed. * Resolves #332 * Merged fix for #330 and added tests that verify it * Fixes bug found from a fix in #281 * Recompiled assets and updated changelog for 1.5.0
Replace element instead of putting stuff inside it. (keep extraClasses)
Correct bug when lengthOfTime is not a month;