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

Added highlevel ability to provide lineCap style #499

Merged
merged 9 commits into from
Jun 26, 2020

Conversation

soadzoor
Copy link
Contributor

Similar to #498 , but for lineCap.

There was a comment from @Hopding , saying:
// TODO: Should assert options.lineCap here, but is a breaking change

I'm not sure why this would be a breaking change, we can just default to LineCapStyle.Butt, or we can ignore this parameter if it's not provided (like this PR does). Maybe there's more to it than I can see, so let me know! :)

Copy link
Owner

@Hopding Hopding left a comment

Choose a reason for hiding this comment

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

Thanks @soadzoor! These changes look good as well. But please take a look at the changes I requested for #498 as some of them apply here. In particular, most usages of lineCap should be renamed to borderLineCap for the sake of consistency.

Peter Varga added 4 commits June 24, 2020 18:30
# Conflicts:
#	apps/deno/tests/test12.ts
#	apps/node/tests/test1.ts
#	apps/node/tests/test12.ts
#	apps/rn/src/tests/test1.js
#	apps/rn/src/tests/test12.js
#	apps/web/test12.html
#	src/api/PDFPage.ts
#	src/api/PDFPageOptions.ts
#	src/api/operations.ts
@soadzoor
Copy link
Contributor Author

@Hopding All done, let me know if it needs anything else! :)

@Hopding
Copy link
Owner

Hopding commented Jun 25, 2020

@soadzoor Could you please merge master into your branch and resolve these conflicts?

# Conflicts:
#	apps/deno/tests/test1.ts
#	apps/deno/tests/test12.ts
#	apps/node/tests/test1.ts
#	apps/node/tests/test12.ts
#	apps/rn/src/tests/test1.js
#	apps/rn/src/tests/test12.js
#	apps/web/test1.html
#	apps/web/test12.html
#	src/api/PDFPage.ts
#	src/api/PDFPageOptions.ts
#	src/api/operations.ts
@soadzoor
Copy link
Contributor Author

@Hopding sure, done!

@Hopding Hopding merged commit 435bddf into Hopding:master Jun 26, 2020
Hopding pushed a commit that referenced this pull request Aug 30, 2021
* Added highlevel ability to provide dashArray, and dashPhase for page.drawLine

* Added dashArray and dashPhase for the rest of the drawing functions: rectangle, square, ellipse, circle, svgpath

* Updated tests to reflect dashpattern changes

* Added highlevel ability to provide lineCapStyle to the following page.draw functions: line, square, rectangle, circle, ellipse

* Applying requested changes

* Updated tests to reflect changes

* Applying requested changes: lineCap to borderLineCap

Co-authored-by: Peter Varga <peter.varga@xyicon.com>
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.

2 participants