Skip to content

Adding options for domParserOptions #79

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from
Closed

Adding options for domParserOptions #79

wants to merge 1 commit into from

Conversation

rntgspr
Copy link

@rntgspr rntgspr commented Dec 3, 2018

Adding options for domParserOptions using Object.assign before send it directly to the parser itself;
Setting up a default value for domParserOptions;

Consider a possible refactoring, I am using github interface to update it.

I believe it closes #62

update:
I give up from this PR because we need a different solution to write raw strings on a React components. I believed to be logic to this repo do it, but actually it did not makes sense, crossing HTML/JSX/React Documentations.

@coveralls
Copy link

coveralls commented Dec 3, 2018

Coverage Status

Coverage decreased (-0.7%) to 98.485% when pulling fc8b224 on rntgspr:master into bfe2023 on remarkablemark:master.

Copy link
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.

Thanks for creating the PR @rntgspr. I added a comment and could you also handle the tasks:

  • Run npm run lint:fix to make the CI build pass
  • Add tests

Adding options for `domParserOptions` using Object.assign before send it directly to the parser itself;
Setting up a default value for `domParserOptions`;

Consider a possible refactoring, I am using github interface to update it.

update:
Now on keyboard, reafactoring made,
Thanks to @remarkablemark about the iE hint!
Tests added.
@remarkablemark
Copy link
Owner

@rntgspr What is the warning that you get?

@remarkablemark
Copy link
Owner

@rntgspr Did you close this PR intentionally? If so, let me know what I can do to help you get this merged in because I believe the feature here is useful

@rntgspr
Copy link
Author

rntgspr commented Dec 10, 2018

Yes,

It can be useful, but it no matches with all documentations together.

1- warnings from ReactJS (warnings, self explanatory);
2- self-close from JSX(it always open and closes custom self-closed tags, it don't works in all possible scenarios);
3- lowercase from HTML5(take a look on: Use Lower Case Element Names section);

best,
Gaspar

@remarkablemark
Copy link
Owner

Thanks for the update @rntgspr. I appreciate the effort and I'll borrow some things from this PR to make as feature enhancements.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't change case of tags
3 participants