Conversation
|
This would be the next nibble on raquo/Laminar#172 I had a formatting nightmare on windows, so I setup a devcontainer instead of working locally. that config could be deleted, although it may also be independently useful. I think this has a shot at addressing the 4 outstanding points that would be needed in SDT |
There was a problem hiding this comment.
Pull Request Overview
This PR continues MathML experimentation by expanding MathML attribute definitions and reorganizing the codebase structure. The changes add comprehensive MathML attribute support to the DOM types library.
- Adds 19 new MathML attribute definitions with proper documentation
- Updates ScalaJS DOM version from snapshot to stable release
- Introduces code generation support for MathML attributes
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| MathMLDefs.scala | Replaced placeholder with comprehensive MathML attribute definitions |
| HtmlAttrDefs.scala | Added commented-out xmlns attribute to avoid clashes |
| CanonicalDefGroups.scala | Added MathML attribute definition groups for code generation |
| Versions.scala | Updated ScalaJS DOM version from snapshot to stable release |
| GeneratorSpec.scala | Added test for generating MathML attributes |
| Attr.scala | Added MathMLAttr class definition |
| MathMLAttrs.scala | Generated MathML attributes trait (new file) |
| CompileSpec.scala | Updated tests to include MathML attributes |
| instructions.md | Added project documentation |
| devcontainer.json | Added development container configuration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| scalaValueType = "String", | ||
| codec = "StringAsIs", | ||
| commentLines = List( | ||
| "This attribute specifies the desired width. See length for possible values.", |
There was a problem hiding this comment.
The 'width' attribute conflicts with HTML's width attribute. Consider using a more specific name like 'mathmlWidth' or 'mspaceWidth' to avoid namespace collisions when both HTML and MathML attributes are used together.
| "This attribute specifies the desired width. See length for possible values.", | |
| scalaName = "mspaceWidth", | |
| domName = "width", | |
| namespace = None, | |
| scalaValueType = "String", | |
| codec = "StringAsIs", | |
| commentLines = List( | |
| "This attribute specifies the desired width for MathML <mspace> elements. See length for possible values.", |
|
|
||
| AttrDef( | ||
| tagType = MathMlTagType, | ||
| scalaName = "height", |
There was a problem hiding this comment.
The 'height' attribute conflicts with HTML's height attribute. Consider using a more specific name like 'mathmlHeight' or 'mspaceHeight' to avoid namespace collisions when both HTML and MathML attributes are used together.
| scalaName = "height", | |
| scalaName = "mathmlHeight", |
|
Something I think I'm realising, is that not all math ml elements are available on all tags, but this doesn't do anything to attempt to acknowledge that. Is that reasonable, or does SDT have some mechanism to say "this attribute is only valid with this tag"? |
|
Heyo, sorry I didn't get back to you yet, I'm rather swamped right now. I should be able to review this in the second half of September. |
|
No problem - there is no rush, but thanks for the message. |
|
Thanks for your work @Quafadas! I have squash-merged the branch of this PR on the command line. Here it is in git history – it appears with your authorship properly attached, the same as if I squash-merged this PR. In retrospect I could have just merged this PR and went from there but TBH there was a lot of stuff I needed to do for MathML in Laminar, and I just didn't think this part through. So, I need to close this PR, but your code & commit & authorship is in the next-v19 branch, which will become the master branch soon. I've been working on finishing MathML support all day today, and it's 99% done. I haven't tested it in the browser yet, only in unit tests. It should work, except that it doesn't support the
And then you can use this version of Laminar locally. I wrote a basic MathMlSpec in Laminar, you can start with that code. This version of Laminar has a bunch of other changes, some of which are binary and source incompatible, but if you just have a small test project with no dependencies that depend on com.raquo stuff then you should probably be ok. If you'd rather wait, I plan to publish v18.0.0-M1 of Laminar in a few weeks, definitely some time this year. |
|
OOOooohhhhh, you already went all the way to Laminar - wow - very exciting! If I can dig out enough time to give it a shot I will... |
No description provided.