-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Use SVG Namespace for SVG Elements #5957
Conversation
Hi @rockerBOO, thanks for the PR. the file Can you explain what exactly will change with this different way to set the attributes? Also i think it would be nice to have the SVG namespace defined on the fabric object like
So that we do not have to repeat it all over the place. |
This change is for consistency. The We apply the namespace in all other examples (through raw SVG files) but the behavior of these tests may change as browser adaptions implement fallback behavior differently. As all the tests pass without changing the tests, we can assume most browsers fallback appropriately currently. I can make the modifications shortly. |
@rockerBOO let me know if you can make the changes, or give me the write auth on the branch i'll move it forward |
I believe I have given you write access. Updating to this would be the last step.
|
i'll do the change on top of your, since is easier to me. Less fiddling with git. |
* Use SVG Namespace for createElement of SVG elements * Updating to use setAttributeNS * Reverting line endings... I hope * Fixing linter errors * Resetting dist/ and removing .gitattributes
createElement
withcreateElementNS
for SVG elements.setAttribute
withsetAttributeNS
for SVG attributes.Doing some code quality checks and found this suggestion.
https://webhint.io/docs/user-guide/hints/hint-create-element-svg/
http://zhangwenli.com/blog/2017/07/26/createelementns/
Side note,
dist/fabric.js
and.gitattributes
are not intentional to this request.