-
-
Notifications
You must be signed in to change notification settings - Fork 11
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 a PNG output to LegendRenderer #393
Conversation
From a first glance, this looks really good! Thanks @jahow Could you let us know, when this is ready for review? |
Sure, hopefully in the coming days! |
Making slow progress on this but I haven't forgotten 🤓 |
0ee5341
to
aa820c8
Compare
@jansule the test coverage is now pretty good, this is ready for review. I couldn't make a test app yet but this is in my plans for later. At least this PR should keep the default behaviour unchanged and not break anything! |
aa820c8
to
fc240fe
Compare
This implements atomic operations like adding text or images, as well as generating the final image.
This means that LegendRenderer does not use SVG or D3 at all and only rely on the output implementation. This change should not change the output or behaviour of LegendRenderer in any way, except for one this: the 'xmlns' attribute is now set every time an image is added to the svg legend, and not only with remote legends.
LegendRenderer can now give an image without a container, and the output type can be changed (SVG by default).
Containers are ignored in PNG, and a custom logic is added to extend the canvas when an image goes outside of the frame.
Remove dependencies to d3, check for output calls instead
fc240fe
to
eb52721
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great work @jahow! Thanks
Co-authored-by: Jan Suleiman <jansule@users.noreply.github.com>
@jansule this is good to merge! |
This PR refactors the
LegendRenderer
class to externalize all "rendering" logic (i.e. using D3 and SVG) to a separateSvgOutput
class with atomic operations (add image, add label, etc.).Then, another
PngOutput
class is added andLegendRenderer
can essentially use both interchangeably. Also added arenderAsImage
method to theLegendRenderer
API in order to obtain an image without providing a container, useful when the legend is intended to be downloaded instead of being added to the DOM.To use PNG output:
The important part is that the default behavior of
LegendRenderer
(generating an SVG and appending it) is almost entirely unchanged. The only difference is about the root svg namespace which is sometimes added when it was not before (see commits).Caveat: labels are not shortened in the PNG output. This will need a bit more refactoring so I thought I'd leave that for later.
Remaining to do:
Also I think it would be nice to set up some wider regression tests so that it's easier to add more features, e.g. automatic width adjustment, more options to customize layout, etc.!
Fixes #390