sayanee / angularjs-pdf

:page_facing_up: An AngularJS directive <ng-pdf> to display PDF files with PDFJS
http://sayan.ee/angularjs-pdf/
MIT License
494 stars 248 forks source link

Es6 #163

Closed simobasso closed 7 years ago

simobasso commented 7 years ago

move whole package to es6

codecov-io commented 7 years ago

Current coverage is 69.46% (diff: 69.46%)

No coverage report found for master at eda56e0.

Powered by Codecov. Last update eda56e0...ecdd2b2

dennybiasiolli commented 7 years ago

@simobasso can we also have an angular-pdf.js file in dist folder after building with webpack, like in the current master?

rhyskoedijk commented 7 years ago

Could I make a small suggestion since you are already writing this. Consider exposing the PDF.js _pdfDoc in the onLoad callback, which would allow end-users to provide their own hooks in to the document. I wanted to explore the adding "find text" functionality to my viewer but as Angular-PDF keeps the document internal, it is impossible to extend currently.

if (angular.isFunction(scope.onLoad)) { scope.onLoad(_pdfDoc); }

Maybe also a new callback at the start of the renderPage function to allow end-users to perform their own extra logic/rendering too?

rhyskoedijk commented 7 years ago

Can I fork simobasso:es6 and submit my own pull requests for this new version, or is that just making things overly complicated? I am interested in:

An issue I had around the pdfTask being cancelled when using multiple instances seems to be resolved in this version already as you have re-scoped the variables within the directive.

dennybiasiolli commented 7 years ago

Thanks for the feedback @rhyskoedijk, this speeds up our testing-before-merging process 😉

Our roadmap on the project is the following:

Starting from tomorrow we will fix conflicts in this PR in order to merge as soon as possible, so you can fork the master branch directly. Your suggestions are great, after PR's merge we will glad to focus our attention to them and if you would like to contribute with PRs we can improve this directive together 😄

Thanks for your help

dennybiasiolli commented 7 years ago

@simobasso what do you think about this branch? It resolves conflicts, fixes sintax errors and provide both standard and minimized output.

rhyskoedijk commented 7 years ago

just merge it ;)

simobasso commented 7 years ago

close in favor of #184