Closed jonathantneal closed 3 years ago
Looks good at the first glance. I'll take a closer look this coming weekend.
Thank you for matching the existing code style. It certainly shouldn't intermix tabs and spaces (most of the project uses tabs), but it should be addressed separately.
Could you include links to Jest and JSDOM issues that motivated you to write this pull request? You've mentioned it in https://twitter.com/jon_neal/status/1405245633049600008.
... and I preserved the strategy of using new CSSOM.CSS... over Object.create (which is done to support for Firefox 3.6, perhaps?).
It should be changed to the modern class syntax, e.g. class CSSConditionRule extends CSSGroupingRule
, but it's fine to do that in a separate commit. I don't intend to support 10-year-old browsers, it's just the sign that the repo hasn't been maintained.
Thank you for contributing, @jonathantneal! Your code carefully matched the existing codebase.
I just published https://www.npmjs.com/package/cssom/v/0.5.0.
If you'd like to contribute any changes related to code style inconsistencies (e.g. indentation) or outdated patterns (new CSSOM.CSS...
), I'll gladly take pull requests.
This change adds the
CSSGroupingRule
andCSSConditionRule
constructors, and it establishes the prototype chain between these andCSSMediaRule
andCSSSupportsRule
.For the updated functionality, it includes references to the appropriate specifications, and I did my best to keep the coding style in sync with the existing project. For instance, I preserved the styling where tabs are currently intermixed with spaces in
CSSMediaRule.js
, and I preserved the strategy of usingnew CSSOM.CSS...
overObject.create
(which is done to support for Firefox 3.6, perhaps?).For the tests, I verified that, while one test is failing from the current main branch, all the other tests are still passing after this change. I included a comment similar to others for a currently untestable getter.
Resolves #105