Skip to content

SVG improvements (rebased) - #3843

Merged
domenic merged 4 commits into
mainfrom
svg2
Mar 23, 2025
Merged

SVG improvements (rebased)#3843
domenic merged 4 commits into
mainfrom
svg2

Conversation

@domenic

@domenic domenic commented Mar 16, 2025

Copy link
Copy Markdown
Member

This is a rebase and partial split-up of #2926, with the goal of landing it.

domenic and others added 2 commits March 16, 2025 16:32
Previously it was being done manually, but now it is done in the reflection layer. This should have no functional changes.

Also adds support for [ReflectDeprecated] being used with namespaced attributes, which will be important for xlink:href.

Co-authored-by: Timothy Gu <timothygu99@gmail.com>
@domenic

domenic commented Mar 16, 2025

Copy link
Copy Markdown
Member Author

Implementation notes for part 1 (AI allowed):

AI was only slightly helpful here. I tried pointing Cursor (default agent mode) at the original PR and asking it to port over changes, but it pretended to do so while instead fabricating new implementations from scratch. (It admitted to doing so after I called it out.) I had more success letting Cursor do some more minor refactoring, like the first commit that moves reflection into its own file.

I briefly used Claude Sonnet 3.7 (non-thinking) to remind myself of how to port changes from one Git branch to another.

domenic and others added 2 commits March 23, 2025 15:07
New elements: <defs> (SVGDefsElement), <desc> (SVGDescElement), <g> (SVGGElement), <metadata> (SVGMetadataElement), <switch> (SVGSwitchElement), <symbol> (SVGSymbolElement), <use> (SVGUseElement).

Add SVGAnimatedPreserveAspectRatio and SVGAnimatedRect, including support in the reflection layer.

Update the SVGElement viewportElement() method to correctly establish the viewport based on ancestor elements.

Add the SVGSVGElement createSVGRect() method, and the SVGRect type (which is distinct from DOMRect.)

Includes tests for most (but not all) of the new functionality.

Co-authored-by: Timothy Gu <timothygu99@gmail.com>
* Add tests for <metadata> and <switch>
* Improve test coverage for <g>, and<symbol>
* Remove redundant SVGAnimatedString, SVGNumber, and SVGStringList tests that already existed in the repository, but are less complete than the tests we included in the previous commit.
* Remove <use> support. The previous commit included a stub of <use>, but <use>'s main behavior is very complicated (involving shadow roots and such), and we should not pretend to support it if we don't support that part.
@domenic

domenic commented Mar 23, 2025

Copy link
Copy Markdown
Member Author

Implementation notes part 2 (AI not allowed):

AI would definitely have been helpful here, but this ended up being less volume of work than I thought, so the loss wasn't so bad. (Honestly, I was kind of dreading this work for the last week because I thought I'd have to grind out tons of repetitive test code without AI.) After realizing that <use> support was not a good idea, that left only two test files to write (for <metadata> and <switch>), and doing them by hand wasn't that bad.

@domenic
domenic merged commit 059c53c into main Mar 23, 2025
@domenic
domenic deleted the svg2 branch March 23, 2025 07:07
This was referenced Mar 23, 2025
@domenic domenic added the metr uplift A candidate for the METR Uplift experiment label Apr 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

metr uplift A candidate for the METR Uplift experiment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant