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

Add HTML Link, used for stylesheets #74

Merged
merged 3 commits into from
Sep 28, 2023
Merged

Conversation

jgreenbaum
Copy link

There didn't seem to be a way to add a tag for stylesheets. Here I add the HtmlLink type (see https://www.w3.org/TR/SVG/styling.html#LinkElement). The typical usage looks like this:

svg_doc = svg_doc.add(HtmlLink::new()
        .set("xmlns", "http://www.w3.org/1999/xhtml")
        .set("rel","stylesheet")
        .set("type","text/css")
        .set("href", "my_stylesheet_url"));

@IvanUkhov
Copy link
Member

IvanUkhov commented Sep 21, 2023

Hm, good point. Thank you. I am at an impasse now. HtmlLink does not feel right. It is very tempting to rename Link to Anchor and HtmlLink to Link. It can potentially screw up things for a lot of people, but provided there would be a version bump, it seems to be the right thing to do. What do you think? Would you be able to update the PR?

@jgreenbaum
Copy link
Author

jgreenbaum commented Sep 22, 2023

I'm exactly of the same mind as you ... it is unfortunate that tag 'a' is named Link, but I worry about breaking the world to change it when it seems there are many users. I chose HtmlLink since that is how the SVG spec describes it and doesn't require an API change.

I'm happy to update the PR at any time.

I'd hate to suggest that adding the tag is so groundbreaking as to inspire a new major version, but that might be the right thing to do if it requires an API breaking change. Are you ready to otherwise call it 1.0? Are there other changes in the queue that would be breaking as well?

@IvanUkhov
Copy link
Member

Not sure if I am ready to call it 1.0. I will increment the minor version for now. So let us break it! 😀

@jgreenbaum
Copy link
Author

I appreciate your enthusiasm! I'll update the PR ...

jgreenbaum added 2 commits September 25, 2023 08:40
As per dicussion with Ivan, rename exisiting "a" tag element
Link to Anchor, so that "link" can be Link.
@jgreenbaum
Copy link
Author

I've updated the pull request, and tested lightly with my application.

@IvanUkhov IvanUkhov merged commit e8723e3 into bodoni:main Sep 28, 2023
@IvanUkhov
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants