-
-
Notifications
You must be signed in to change notification settings - Fork 134
refactor(utilities): refactoring Utilities constants #1003
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
refactor(utilities): refactoring Utilities constants #1003
Conversation
…-parser into refac/utilities
…act-parser into refac/utilities
|
||
var RESERVED_SVG_MATHML_ELEMENTS = new Set([ | ||
'annotation-xml', | ||
'color-profile', | ||
'font-face', | ||
'font-face-src', | ||
'font-face-uri', | ||
'font-face-format', | ||
'font-face-name', | ||
'missing-glyph' | ||
]); | ||
|
||
function isCustomComponent(tagName, props) { | ||
if (tagName.indexOf('-') === -1) { | ||
return props && typeof props.is === 'string'; | ||
} | ||
|
||
switch (tagName) { | ||
// These are reserved SVG and MathML elements. | ||
// We don't mind this whitelist too much because we expect it to never grow. | ||
// The alternative is to track the namespace in a few places which is convoluted. | ||
// https://w3c.github.io/webcomponents/spec/custom/#custom-elements-core-concepts | ||
case 'annotation-xml': | ||
case 'color-profile': | ||
case 'font-face': | ||
case 'font-face-src': | ||
case 'font-face-uri': | ||
case 'font-face-format': | ||
case 'font-face-name': | ||
case 'missing-glyph': | ||
return false; | ||
default: | ||
return true; | ||
// These are reserved SVG and MathML elements. | ||
// We don't mind this whitelist too much because we expect it to never grow. | ||
// The alternative is to track the namespace in a few places which is convoluted. | ||
// https://w3c.github.io/webcomponents/spec/custom/#custom-elements-core-concepts | ||
if (RESERVED_SVG_MATHML_ELEMENTS.has(tagName)) { | ||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I worked similarly to the existing "elementsWithNoTextChildren" approach. 🙏
Codecov Report
@@ Coverage Diff @@
## master #1003 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 8 8
Lines 177 169 -8
Branches 59 50 -9
=========================================
- Hits 177 169 -8
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@remarkablemark |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ssi02014! LGTM
What is the motivation for this pull request?
Hello 👋, @remarkablemark
I worked on refactoring by isolating code that could be managed by constants inside the
utilities.js
.Please review this pull request to see if it's appropriate. 🙏
What is the current behavior?
There is no change in behavior.
What is the new behavior?
There is no change in behavior.
Checklist: