Refactor to linkifyjs! #22
Conversation
|
Oh, just noticed the readme needs updating. Will get onto that... should probably do that before any publishing! |
| var options = angular.extend(defaultOptions, scope.options); | ||
|
|
||
| scope.$evalAsync(function () { | ||
| try { |
There was a problem hiding this comment.
There's a bizarre error on their part where if it finds just one text node that's empty, it errors and breaks the module
|
@marksyzm so far, this is looking good! My number 1 concern in this PR is the the change it makes to the size of the bundle. It seems to almost quadruple it. I'm actually ok with that if it solves a more general problem. Also, when you get the tests in, make sure they run against the old code as well as the new code (at least the functionality of the old code). That way we can be confident that there aren't regressions. Thanks again! |
|
Well, the requirement of linkifyjs is basically the issue, there, size-wise, because it's bundled into the build script. I don't think there's a way around that when it's being put into real use, even if I manage to get the build to remove linkifyjs scripts and just bundle up the modules, but at least you get the new configurable set up (worth a look at: http://soapbox.github.io/linkifyjs/docs/options.html). If someone were to use this via bower, they would use the built plugin, else NPM via webpack or similar means they can separately install versions of linkifyjs in their What's also nice there is that you can chuck in plugins of your own too by using their plugin system. |
|
As for the tests, do you mean create a separate tag for the old stuff and create tests based on those also? The old stuff is no longer there in this build; I was just going to create tests for the new services, directive and filter otherwise. |
|
I had to refactor to allow for a new system where it just takes in a string as first argument for each, and an object for second which matches up with the format that <!-- filter -->
<div ng-bind-html="{{ 'www.google.com' | linkify: { nl2br: true, defaultProtocol: 'https' } }}"></div>
<!-- directive -->
<div linkify>{{'www.google.com'}}</div> |
Remove old modules.
Create build process for publishing.
Add new linkify integration with angular.