Skip to content

support custom styles beginning with '--*'#83

Merged
remarkablemark merged 1 commit intoremarkablemark:masterfrom
samikarak:master
Dec 17, 2018
Merged

support custom styles beginning with '--*'#83
remarkablemark merged 1 commit intoremarkablemark:masterfrom
samikarak:master

Conversation

@samikarak
Copy link
Copy Markdown
Contributor

@samikarak samikarak commented Dec 14, 2018

See #82

Resolves #81

@coveralls
Copy link
Copy Markdown

coveralls commented Dec 14, 2018

Coverage Status

Coverage increased (+0.005%) to 99.286% when pulling 2c0a9dc on samikarak:master into 185d2f2 on remarkablemark:master.

@samikarak
Copy link
Copy Markdown
Contributor Author

samikarak commented Dec 14, 2018

@remarkablemark I updated the pull request like described in the previous one, would you mind merging your current changes on master branch into it ?

The previous PR somehow got deleted after a rebase. I just commented there, that the regex you proposed did not work for custom styles containing dashes in between e.g '--custom-style' would get transformed into '-CustomStyle' since the regex would return false.

// 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;
  }

  // ...

Now I just replaced string.startsWith('--') with string.indexOf('--') === 0.

function camelCase(string) {
  // ...

  // customStyle or no hyphen found
  if (string.indexOf('--') === 0 || string.indexOf('-') === -1) {
    return string;
  }

  // ...

Copy link
Copy Markdown
Owner

@remarkablemark remarkablemark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mind updating your commit messages to use the conventional commit format?

$ git commit --amend -m 'feat(utilities): add support for custom styles beginning with "--*"'
$ git push origin master --force

It uses the commit type to help with releasing (otherwise, the version bump wouldn't be correct). (The precommit hook checks this.)

Also, do you mind adding a test?


Tasks

  • Amend commit message
  • Add test

lib/utilities.js Outdated
// no hyphen found
if (string.indexOf('-') === -1) {
// customStyle or no hyphen found
if (string.indexOf('--') === 0 || string.indexOf('-') === -1) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I asked in #82, do you think it's useful making the declaration value check strict here?

-var CUSTOM_PROPERTY_OR_NO_HYPHEN_REGEX = /^--[a-zA-Z]+$|^[^-]+$/;
+var CUSTOM_PROPERTY_OR_NO_HYPHEN_REGEX = /^--[a-zA-Z-]+$|^[^-]+$/;

Copy link
Copy Markdown
Contributor Author

@samikarak samikarak Dec 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your modified regex seems to work perfectly, so replaced the strict check with your regex matcher. I also added the tests and modified the commit message as preferred.

Thank you for your help !

@remarkablemark remarkablemark added the feature New feature or request label Dec 17, 2018
@remarkablemark remarkablemark merged commit 1f26283 into remarkablemark:master Dec 17, 2018
d-lazarev pushed a commit to d-lazarev/html-react-parser that referenced this pull request Apr 5, 2019
support custom styles beginning with '--*'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants