Skip to content
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

Add attribute to limit the canvas-height to its container #128

Closed
wants to merge 2 commits into from

Conversation

lesteenman
Copy link

This patch adds an attribute that, combined with scale='page-fit',
allows to fit the contents inside of the page both with respects to the
width and with respect to the height. Think of it as using
background-size='contain' rather than the default behavior of
background-size='cover' that scale='page-fit' triggers.

Fixes #127.

@lesteenman
Copy link
Author

WIP: Pushed too early.

@lesteenman
Copy link
Author

Note: It seems that even the cleanly pulled branch does not pass the tests, with the same error I'm getting here. However, I see that Travis still says your own builds are passing. Am I doing something wrong?

@sayanee
Copy link
Owner

sayanee commented May 11, 2016

Thanks @lesteenman - I haven't look at this in detail, but could you see some instructions on CONTRIBUTING.md to run some tests / build locally with grunt and npm? That might give you some clue hopefully!

@lesteenman
Copy link
Author

lesteenman commented May 11, 2016

Yes, I have. I'm having no issues with steps 1-4. However, when I run npm run build, I'm getting the following error:

Running "karma:unit" (karma) task
11 05 2016 12:32:43.285:INFO [karma]: Karma v0.13.22 server started at http://localhost:9876/
11 05 2016 12:32:43.298:INFO [launcher]: Starting browser PhantomJS
11 05 2016 12:32:43.682:INFO [PhantomJS 1.9.8 (Linux 0.0.0)]: Connected on socket /#EIIhO8EZAS2tWGgcAAAA with id 34226259
PhantomJS 1.9.8 (Linux 0.0.0) ERROR
  TypeError: '[object Uint8Array]' is not a valid argument for 'Function.prototype.apply' (evaluating 'argsArray[i]')
  at /home/erik/projects/angularjs-pdf/example/js/lib/pdf.js:4519
PhantomJS 1.9.8 (Linux 0.0.0): Executed 2 of 4 ERROR (0.948 secs / 0.013 secs)
Warning: Task "karma:unit" failed. Use --force to continue.

Note that this same error occurs with a clean clone of your repository, and the same error occurs with the Travis CI build. Running npm test also gives this error.

@lesteenman
Copy link
Author

Updating karma-phantomjs-launcher to ^1.0.0 (from ^0.2.1) seems to have worked. I'll submit a new PR.

This patch adds an attribute that, combined with `scale='page-fit'`,
allows to fit the contents inside of the page both with respects to the
width and with respect to the height. Think of it as using
`background-size='contain'` rather than the default behavior of
`background-size='cover'` that `scale='page-fit'` triggers.
@lesteenman lesteenman force-pushed the restrictCanvasHeight branch from edc4045 to 79082f0 Compare May 11, 2016 13:24
@lesteenman
Copy link
Author

This new version does not include phantomjs-prebuilt anymore. Adding a commit to include this dependency manually.

@dennybiasiolli
Copy link
Collaborator

@lesteenman can you please rebase this PR? Now the problems with tests should be resolved.

@lesteenman
Copy link
Author

The earliest I can do that is next Tuesday, probably. If there is a deadline before then, please do so yourself; the changes should be straightforward.

Copy link
Collaborator

@dennybiasiolli dennybiasiolli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please apply changes to src/angular-pdf.js instead of dist/angular-pdf.js, then commit only that file with readme.md and some tests/examples

simobasso added a commit to simobasso/angularjs-pdf that referenced this pull request May 25, 2017
allows to fit the contents inside of the page both with respects to the
width and with respect to the height. Think of it as using
`background-size='contain'` rather than the default behavior of
`background-size='cover'` that `scale='page-fit'` triggers.

thanks to @lesteenman, original author of this commit.

close sayanee#128 and close sayanee#127
dennybiasiolli added a commit that referenced this pull request May 25, 2017
This patch adds an attribute that, combined with `scale='page-fit'`,
allows to fit the contents inside of the page both with respects to the
width and with respect to the height. Think of it as using
`background-size='contain'` rather than the default behavior of
`background-size='cover'` that `scale='page-fit'` triggers.

thanks to @lesteenman, original author of this commit.

close #128 and close #127
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants