Closed karras closed 5 months ago
@bocytko Any chance to get this merged? :)
Currently the radar SVG has its font-family hardcoded, with this PR it becomes customizable.
Thanks for pointing out the missing configuration. What's the likelihood of a font-family change requiring a subsequent font-size change? In the end also the magic number offsets are related to the font sizes, which would complicate matters.
I noticed that
radar.css
prefers Source Sans Pro for the surrounding text. Let me know if I should also adjust that or you'd like to handle it separately.
Please open an issue for that and suggest how it could be achieved. If it does not end up being too ugly, I'd consider accepting a PR.
Thanks for pointing out the missing configuration. What's the likelihood of a font-family change requiring a subsequent font-size change? In the end also the magic number offsets are related to the font sizes, which would complicate matters.
Good point, I guess with some font-families it might lead to issues, and as you pointed out making all the font-size combinations adjustable leads to more complexity. Maybe I could add a comment indicating that changing the font-family might lead to subsequent readability challenges ("change at your own risk")?
I noticed that
radar.css
prefers Source Sans Pro for the surrounding text. Let me know if I should also adjust that or you'd like to handle it separately.Please open an issue for that and suggest how it could be achieved. If it does not end up being too ugly, I'd consider accepting a PR.
Here a short comparison between Arial and Source Sans Pro:
Good point, I guess with some font-families it might lead to issues, and as you pointed out making all the font-size combinations adjustable leads to more complexity. Maybe I could add a comment indicating that changing the font-family might lead to subsequent readability challenges ("change at your own risk")?
Short comment sounds good. You can also mention 1-2 alternate font families that are compatible with current settings.
Apologies for the delay, added a comment to the example in the README and defined a default value.
@bocytko What do you think? Anything left to do from my side? :)
@karras looks good now, thanks for the PR and your patience.
Currently the radar SVG has its font-family hardcoded, with this PR it becomes customizable.
I noticed that
radar.css
prefers Source Sans Pro for the surrounding text. Let me know if I should also adjust that or you'd like to handle it separately.