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

Provide better tests for SVG attributes #990

Merged
merged 6 commits into from
Dec 20, 2023
Merged

Provide better tests for SVG attributes #990

merged 6 commits into from
Dec 20, 2023

Conversation

Elchi3
Copy link
Member

@Elchi3 Elchi3 commented Dec 18, 2023

This PR fixes some SVG attribute tests:

  • All xlink_href, xlink_actuate, xlink_show, and xlink_title tests need special handling and so I updated the element test builder. They are used on many SVG elements.
  • Several attributes have different names in the SVGOM. I did some replacements in the test builder as they appear many times on various SVG elements as well.

@queengooborg queengooborg temporarily deployed to mdn-bcd-collector-pr-990 December 19, 2023 16:53 Inactive
@Elchi3 Elchi3 changed the title Add tests for xlink attributes Provide better tests for SVG attributes Dec 19, 2023
@queengooborg queengooborg temporarily deployed to mdn-bcd-collector-pr-990 December 19, 2023 17:39 Inactive
test-builder/elements.ts Outdated Show resolved Hide resolved
custom/tests.yaml Outdated Show resolved Hide resolved
@Elchi3
Copy link
Member Author

Elchi3 commented Dec 19, 2023

oh err "'attrProp' is of type 'unknown'." says Typescript now. I guess I need to tell it a type somehow?

@queengooborg
Copy link
Member

Argh, f#&$ing TypeScript... Unfortunately, it doesn't attribute types for Object.entries, which causes lots of issues. If we specify Object.entries(...) as [string, string][], that should fix it?

@queengooborg queengooborg temporarily deployed to mdn-bcd-collector-pr-990 December 19, 2023 20:04 Inactive
@Elchi3
Copy link
Member Author

Elchi3 commented Dec 19, 2023

If we specify Object.entries(...) as [string, string][], that should fix it?

I was toying around to figure out how to assign the string type, but I had no idea it is as [string, string][] after Object.entries. Thanks much for the hint!

Co-authored-by: Queen Vinyl Da.i'gyu-Kazotetsu <vinyldarkscratch@gmail.com>
@queengooborg queengooborg temporarily deployed to mdn-bcd-collector-pr-990 December 20, 2023 08:55 Inactive
@queengooborg queengooborg merged commit 742fc84 into main Dec 20, 2023
5 checks passed
@queengooborg queengooborg deleted the xlink-tests branch December 20, 2023 17:35
@Elchi3
Copy link
Member Author

Elchi3 commented Dec 20, 2023

I was about to comment on this PR if I should have updated element.json instead of implementing replacements here.
Basically what I did in #997 (which I just learned about!)

@queengooborg
Copy link
Member

Oh! Yes, that would probably be the better option for the non-xlink attributes...I forgot that was a thing, and I'm the one who implemented it! 😆

@Elchi3
Copy link
Member Author

Elchi3 commented Dec 20, 2023

Working on a PR to use that instead!

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