karaxnim / karax

Karax. Single page applications for Nim.
MIT License
1.05k stars 90 forks source link

Update karax.nim #256

Closed scattergory closed 8 months ago

scattergory commented 1 year ago

use tests for mathML and SVG nodes to make these elements write to the DOM correctly by default

geotre commented 1 year ago

Hi @scattergory thanks for the PR. There have been a few attempts previously at adding SVG/namespace support, so I would like to check back on those to understand the changes they were looking to add.

It would also be good to add tests to ensure that it works correctly.

Please ping me when it is ready to check (if you can unify #255 in as well please) and I will look over this as soon as possible

choltreppe commented 8 months ago

Hey, I'd like to push this PR over the finish line, how can I do that ?

Araq commented 8 months ago

Error: execution of an external program failed: '/__w/karax/karax/tests/tester '
.......................................
/__w/karax/karax/karax/kdom.nim(9, 10) Hint: duplicate import of 'dom'; previous import here: /__w/karax/karax/karax/kdom.nim(3, 6) [DuplicateModuleImport]
/__w/karax/karax/karax/kdom.nim(3, 6) Warning: imported and not used: 'dom' [UnusedImport]
.......................
/__w/karax/karax/karax/vstyles.nim(329, 22) Warning: implicit conversion to 'cstring' from a non-const location: aj[0]; this will become a compile time error in the future [CStringConv]
/__w/karax/karax/karax/vstyles.nim(329, 29) Warning: implicit conversion to 'cstring' from a non-const location: aj[1]; this will become a compile time error in the future [CStringConv]
.
/__w/karax/karax/karax/vdom.nim(72, 3) Hint: 'svgElements' is declared but not used [XDeclaredButNotUsed]
/__w/karax/karax/karax/vdom.nim(74, 3) Hint: 'mathElements' is declared but not used [XDeclaredButNotUsed]
.....
/__w/karax/karax/karax/karax.nim(199, 8) Error: undeclared identifier: 'svgElements'

Remember that parts of the Karax DOM also work with the native backends.

geotre commented 8 months ago

I think we can close this PR as it has been superseded by #261