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

Adding support to style attributes in SVG files #419

Merged
merged 12 commits into from
May 9, 2022
Merged

Adding support to style attributes in SVG files #419

merged 12 commits into from
May 9, 2022

Conversation

RedShy
Copy link

@RedShy RedShy commented May 8, 2022

I implemented the support for SVG with CSS styling elements simply by parsing the style attribute and inserting the key-value pairs as attributes of the element and then reusing the existing code. Close #404

I tested it using the SVG files downloadable from Ghostscript examples and it works! There are some with linear gradient not supported but the others seems to be okay.
I added some tests in test_svg.py to cover the new feature.

I also have added simple tests for the file transitions.py given that was the least covered with the aim of increase test coverage.

Checklist:

  • The GitHub pipeline is OK (green),
    meaning that both pylint (static code analyzer) and black (code formatter) are happy with the changes of this PR.

  • A unit test is covering the code added / modified by this PR

  • This PR is ready to be merged

  • In case of a new feature, docstrings have been added, with also some documentation in the docs/ folder

  • A mention of the change is present in CHANGELOG.md

  • The PR description or comment contains a picture of a cute animal (not mandatory but encouraged 😉)

@RedShy RedShy requested a review from Lucas-C as a code owner May 8, 2022 15:00
fpdf/svg.py Outdated Show resolved Hide resolved
test/svg/parameters.py Show resolved Hide resolved
@codecov
Copy link

codecov bot commented May 9, 2022

Codecov Report

Merging #419 (3d7cf4d) into master (3b442f1) will increase coverage by 0.13%.
The diff coverage is 92.85%.

❗ Current head 3d7cf4d differs from pull request most recent head 12c92d1. Consider uploading reports for the commit 12c92d1 to get more accurate results

@@            Coverage Diff             @@
##           master     #419      +/-   ##
==========================================
+ Coverage   91.19%   91.32%   +0.13%     
==========================================
  Files          22       22              
  Lines        6347     6238     -109     
  Branches     1282     1253      -29     
==========================================
- Hits         5788     5697      -91     
+ Misses        315      305      -10     
+ Partials      244      236       -8     
Impacted Files Coverage Δ
fpdf/svg.py 97.50% <92.85%> (-0.13%) ⬇️
fpdf/prefs.py 90.90% <0.00%> (-1.95%) ⬇️
fpdf/enums.py 98.91% <0.00%> (-0.29%) ⬇️
fpdf/drawing.py 93.13% <0.00%> (-0.26%) ⬇️
fpdf/fpdf.py 87.73% <0.00%> (-0.02%) ⬇️
fpdf/__init__.py 100.00% <0.00%> (ø)
fpdf/transitions.py 94.93% <0.00%> (+24.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b442f1...12c92d1. Read the comment docs.

@Lucas-C
Copy link
Member

Lucas-C commented May 9, 2022

Note that currently the GitHub Actions pipeline is failing because your code is not formatted with black:
https://pyfpdf.github.io/fpdf2/Development.html#code-auto-formatting

Just run black . in the repository root directory to fix this.

@Lucas-C
Copy link
Member

Lucas-C commented May 9, 2022

@allcontributors please add @RedShy for code

@allcontributors
Copy link

@Lucas-C

I've put up a pull request to add @RedShy! 🎉

@Lucas-C
Copy link
Member

Lucas-C commented May 9, 2022

This looks great!
Thank you for implementing this, and for adding unit tests!

There are few remaining things that would be great to add to this PR:

  • a test checking that the correct order of priority is implemented between attributes from style & SVG attributes: check https://stackoverflow.com/a/36755226/636849
  • a mention of this improvement & your name in an Added section in CHANGELOG.md: take inspiration from the previous entries 😊
  • keeping the documentation up-to-date, and especially the page about SVG support & limitations: I think the part about style-based limitations could be removed

@Lucas-C
Copy link
Member

Lucas-C commented May 9, 2022

Merging now, thank you for your contribution @RedShy!

@Lucas-C Lucas-C merged commit 92f2001 into py-pdf:master May 9, 2022
@RedShy
Copy link
Author

RedShy commented May 9, 2022

Thank you! it's very rewarding having someone that is supportive and reviews so fast on the other hand!

@Lucas-C
Copy link
Member

Lucas-C commented May 9, 2022

Thanks 😊
I see it as part of the role of a FLOSS project maintainer.

I hope you liked making this first contribution to an open source project,
and that you will do many others!

@Lucas-C
Copy link
Member

Lucas-C commented Jun 17, 2022

This feature has been included in the new v2.5.5 release!

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.

New feature: provide support for style attributes in SVG files
2 participants