add support for custom styles beginning with "--*"#82
add support for custom styles beginning with "--*"#82samikarak wants to merge 0 commit intoremarkablemark:masterfrom
Conversation
remarkablemark
left a comment
There was a problem hiding this comment.
Thanks for opening the PR @samikarak. Do you mind resolving the comments and adding tests?
lib/utilities.js
Outdated
|
|
||
| // no hyphen found | ||
| if (string.indexOf('-') === -1) { | ||
| // customStyle or no hyphen found |
There was a problem hiding this comment.
chore: s/customStyle/custom properties?
lib/utilities.js
Outdated
| // no hyphen found | ||
| if (string.indexOf('-') === -1) { | ||
| // customStyle or no hyphen found | ||
| if (string.startsWith('--') || string.indexOf('-') === -1) { |
There was a problem hiding this comment.
fix: do you mind testing the match using regex since startsWith is not supported in IE?
// regex that matches custom property or no hyphen
var CUSTOM_PROPERTY_OR_NO_HYPHEN_REGEX = /^--[a-zA-Z]+$|^[^-]+$/;
function camelCase(string) {
// ...
if (CUSTOM_PROPERTY_OR_NO_HYPHEN_REGEX.test(string)) {
return string;
}
// ...There was a problem hiding this comment.
seems to not work with custom styles having lisp-case/dash-case in between ...
for example:
CUSTOM_PROPERTY_OR_NO_HYPHEN_REGEX.test('customStyle') // returns : true
CUSTOM_PROPERTY_OR_NO_HYPHEN_REGEX.test('--customStyle') // returns : true
CUSTOM_PROPERTY_OR_NO_HYPHEN_REGEX.test('--custom-style') // returns : false
I think it should also be possible to use custom style having dashes in between (e.g. '--custom-style'). Therefore lets consider an easier approach:
// customStyle or no hyphen found
if (string.indexOf('--') === 0 || string.indexOf('-') === -1) {
return string;
}
It could be possible to use a regex for this check, or to directly replace the string by camelCase if needed, but since javascript does not fully support lookbehind-assertions, the regex gets a little tricky ...
There was a problem hiding this comment.
Good point about the lisp-case. What about the modified regex?
-var CUSTOM_PROPERTY_OR_NO_HYPHEN_REGEX = /^--[a-zA-Z]+$|^[^-]+$/;
+var CUSTOM_PROPERTY_OR_NO_HYPHEN_REGEX = /^--[a-zA-Z-]+$|^[^-]+$/;The indexOf also works, but do you think it's worth making the declaration value check strict?
|
Just a heads up, I pushed some commits to $ git pull --rebase https://github.com/remarkablemark/html-react-parser.git master
$ git push origin master --force |
|
Also, do you mind updating your commit messages to use the conventional commit format? It uses the commit type to help with releasing (otherwise, the version bump wouldn't be correct). |
|
@samikarak Did you accidentally close this PR? |
|
I had to rebase my fork (I had some issues with some workspace files) and requested another PR. This PR got closed automatically. Lets continue on the new PR |
|
Sounds good, thanks for the update @samikarak |
see #81