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

Fix Svg.animate type, name and attributes #308

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

rand00
Copy link

@rand00 rand00 commented Jun 21, 2022

No description provided.

@rand00
Copy link
Author

rand00 commented Jun 21, 2022

I needed to change the type of Xml to include a Semicolon separator - which demands change to js_of_ocaml-tyxml too. I'll push a PR there too

@rand00
Copy link
Author

rand00 commented Jun 21, 2022

This PR includes #306

@Drup
Copy link
Member

Drup commented Jun 21, 2022

The change to the signature of Xml.S is a rather big breaking change (it breaks not only js_of_ocaml, but also eliom, and all users of the functorial API).

I feel in this case, maybe it's for the best to first do all the renaming and deprecations, make a small release with all the other accumulated small fixes, then make a new big release (with some other breaking changes I accumulated).

@rand00
Copy link
Author

rand00 commented Jun 21, 2022

Okay - but one can't use the Svg.animate with animation-values and key-times without this major change; so makes Svg.animate quite useless anyway. Could postpone the renaming because of that?

@Drup
Copy link
Member

Drup commented Jun 21, 2022

Well, you can always implement it in term of string_attrib. It has less good properties, but it should be sufficient in practice for now.

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.

2 participants