Fix backend_svg.RendererSVG.draw_text to render urls#2521
Fix backend_svg.RendererSVG.draw_text to render urls#2521tacaswell merged 1 commit intomatplotlib:masterfrom
Conversation
|
Looks good. Can you add a test and a what's new entry? |
|
Thanks, and sure -- added a test and what's new entry. I'm not familiar with the what's new entries' tone and levels, so I can re-do that in a different format / tone if you let me know an example to follow or any other specifics you'd like. |
There was a problem hiding this comment.
This is crying out for a context manager. Not in scope for this PR though.
There was a problem hiding this comment.
Yeah -- the original commit that introduced the XML writer used a context manager, until I remembered we were still supporting Python 2.5 at the time. Now that we're not, I agree all of backend_svg should be refactored in that style, but not for this PR.
|
👍 - LGTM |
There was a problem hiding this comment.
Can you fix the pep8 violation?
There was a problem hiding this comment.
Sorry, I don't immediately see the PEP 8 violation in the .rst file; did you mean the two newlines after the block of text (lines 62-65)? Happy to format the whats_new text in whatever is the preferred way. As this is my first time and there appear to be different styles for items of differing importance, I had to wing it a bit.
There was a problem hiding this comment.
blah, sorry right line wrong file.
|
👍 from me, once the other concerns are addressed. |
There was a problem hiding this comment.
The pep8 violation is that this line is too long (83 char).
There was a problem hiding this comment.
Thanks - fixed and DRYed. I omitted the return value assignment and tweaked the whitespace a bit for clarity (I hope). Any other feedback very welcome.
|
The PEP8 issue has been addressed. |
|
Can you re-base? This will no longer merge cleanly. |
|
Sure, rebased against matplotlib master. |
|
Travis says there is still issues in |
|
needs another rebase (probably conflicts in 'whats_new.rst') |
|
@mdengler Can you rebase this onto current master again? |
|
Rebased and addressed the Travis issues (hopefully). |
The SVG backend does nothing with Text object's URLs. In particular, the rendering path below backend_svg.RendererSVG.draw_text does nothing with the provided GraphicsContext's _url field.
|
Travis looks good. |
Fix backend_svg.RendererSVG.draw_text to render urls
|
Thanks very much for the merge. |
4 participants
The SVG backend does nothing with Text object's URLs. In particular, the rendering path below
backend_svg.RendererSVG.draw_text does nothing with the provided GraphicsContext's _url field.
This commit fixes that issue, causing Text objects' URLs to become <a> tags around the rendered text in the rendered SVG.