feat: implement Element.getElementsByClassName

[censujiang]

• https://dom.spec.whatwg.org/#dom-element-getelementsbyclassname
• https://developer.mozilla.org/en-US/docs/Web/API/Element/getElementsByClassName

[censujiang]

This bug is fix for #582

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (14e96bc) 93.89% compared to head (6a5b253) 93.96%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #584      +/-   ##
==========================================
+ Coverage   93.89%   93.96%   +0.06%     
==========================================
  Files           8        8              
  Lines        2048     2070      +22     
  Branches      532      534       +2     
==========================================
+ Hits         1923     1945      +22     
  Misses        125      125              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[karfau]

Thank you for the effort.
Could you please translate the comments in the code to English?
It would also be very helpful if you can point to the specification that describes that Element and LiveNodeList have this method. The links should be part of the jsdoc comments and the PR description.

I would also consider this a PR that adds a feature, and not as a fix, since xmldom did not support this method on these Node types before.

Last but not least we will have to add test cases for all the newly supported code paths.

[censujiang]

Thank you for the effort. Could you please translate the comments in the code to English? It would also be very helpful if you can point to the specification that describes that Element and LiveNodeList have this method. The links should be part of the jsdoc comments and the PR description.

I would also consider this a PR that adds a feature, and not as a fix, since xmldom did not support this method on these Node types before.

Last but not least we will have to add test cases for all the newly supported code paths.

I still don’t know why adding the function there can be successful. I just modified it based on observing the structure and general working principle of LiveNode, and after repeated testing on my project, I found that my approach was fine.I'm more curious why it works if I do this

[censujiang]

I will try my best to modify the relevant comments in the code, but I do not guarantee that this will comply with the specifications of this project.

[censujiang]

I have modified the relevant comment operations, but because I cannot write test code, I cannot submit the test code to you.

[karfau]

Can you help me to understand why you can not add tests?

[censujiang]

Can you help me to understand why you can not add tests?

Because I have never written test code in my life...😥

[karfau]

Ok, but do you want to learn it?
Not that this repository is a simple one to get onboarded to,
so maybe you want to get started with testing by following a tutorial like this one to get a general understanding:
https://dev.to/dsasse07/a-beginner-s-guide-to-unit-testing-with-jest-45cc

There are plenty of existing tests in this repository to look at and of course you don't need to configure jest in this repo, you need to run npm ci once to install the dependencies if you havent done it already, and run npm test to run all tests once or npm test -- --watch to rerun the ones related to your changes whenever you save a file.

The tests for Document.getElementsByClassName:

xmldom/test/dom/document.test.js

Line 46 in 14e96bc

describe('getElementsByClassName', () => {

should be a good starting point for what needs to be added to the tests for Elements:

xmldom/test/dom/element.test.js

Line 126 in 14e96bc

describe('Element', () => {

With the difference that you need to call the method on an element node instead of on the documentElement.

Do you want to give it a try?

PS: please use npm run format before commiting and pushing the code, to get rid of the errors in the lint checks.

[censujiang]

I tried to write a little test code and it currently works

[karfau]

That is a great starting point.

When I'm back at my computer next week I will have a closer look at your implementation an potentially mising tests or possible refactoring in the context of this change.
I have not found any spec mentioning that LiveNodeList should implement this method.

If you have a resource that shows this, please provide a link.

[censujiang]

That is a great starting point.

When I'm back at my computer next week I will have a closer look at your implementation an potentially mising tests or possible refactoring in the context of this change. I have not found any spec mentioning that LiveNodeList should implement this method.

If you have a resource that shows this, please provide a link.

okay, Thank you!