Skip to content

Update README with advanced usage of replace method #17

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

Merged
merged 1 commit into from
Aug 30, 2016
Merged

Update README with advanced usage of replace method #17

merged 1 commit into from
Aug 30, 2016

Conversation

poacher2k
Copy link
Contributor

@poacher2k poacher2k commented Aug 29, 2016

Add example to README.md to show use case where children needs to be kept or has its own handler in the replace method.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 89.474% when pulling 633bc14 on poacher2k:master into ddd289b on remarkablemark:master.

@remarkablemark
Copy link
Owner

@poacher2k Thanks for the pull request!

I'm thinking about making a directory examples/ that contains the parser use cases.

What are your thoughts?

style: { fontSize: '42px' } },
parsedChildren);
// equivalent jsx syntax:
// return <span key={key} style={{ fontSize: '42px' }}>{parsedChildren}</span>;
Copy link
Owner

Choose a reason for hiding this comment

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

Could you refactor this part to:

return React.createElement('span', {
    key: key,
    style: { fontSize: '42px' },
}, parsedChildren);
// equivalent jsx syntax
// return <span key={key} style={{ fontSize: '42px' }}>{parsedChildren}</span>;

Just to make the indenting a bit more readable.

@poacher2k
Copy link
Contributor Author

Sure thing! And an example-folder sounds like a good idea.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 89.474% when pulling 52dd2e8 on poacher2k:master into ddd289b on remarkablemark:master.

@remarkablemark
Copy link
Owner

Nice! Lastly, could I trouble you to perform a squash of your commits into a single descriptive commit?

I could perform a squash merge via GitHub but you'll need do some extra stuff to sync up your fork. Let me know what's easier for you.

@remarkablemark remarkablemark changed the title Adds example that keeps children Update README with advanced usage of replace method Aug 29, 2016
@poacher2k
Copy link
Contributor Author

That sounds like a good exercise in broadening my git abilities! It's getting late in Norway, so if it's okay with you, I'll see if I can do this tomorrow after work. If not, go ahead and squash merge it :)

@remarkablemark
Copy link
Owner

Sure, let's finalize it tomorrow. And much appreciated for the PR!

var html = '<div><p id="main"><span class="prettify">keep me and make me pretty!</span></p></div>';

var parserConfig = {
replace: function(domNode, key) {
Copy link
Owner

Choose a reason for hiding this comment

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

I ended up removing key parameter in #18 as I realized there's a better and easier way of doing things.

If you would be so kind, could you remove key in your examples before performing the squash? Thanks so much for dealing with these changes and requests.

After your PR gets merged, I plan to do a release.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 89.623% when pulling 7814369 on poacher2k:master into ddd289b on remarkablemark:master.

Keeping child nodes that in turn gets parsed recursively.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 89.623% when pulling 58f0575 on poacher2k:master into 11770d9 on remarkablemark:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 89.623% when pulling 6a664de on poacher2k:master into 11770d9 on remarkablemark:master.

@poacher2k
Copy link
Contributor Author

I think I managed to do the rebasing correctly! Hope this looks good :)

@remarkablemark
Copy link
Owner

remarkablemark commented Aug 30, 2016

Terrific job @poacher2k.

And thanks for the PR! I'll be doing a release another time because I just did one for 11770d9.

@remarkablemark remarkablemark merged commit 9241f1b into remarkablemark:master Aug 30, 2016
@poacher2k
Copy link
Contributor Author

Awesome, thank you! Probably wouldn't have learned about (super-useful) squashing and rebasing hadn't it been for this. Cheers!

@remarkablemark remarkablemark added the documentation Improvements or additions to documentation label Aug 31, 2016
remarkablemark added a commit that referenced this pull request Sep 27, 2016
- Update README (#17 and #21):
  - Improve documentation of `replace` option with the help of
    @poacher2k
  - Tidy and reword some parts
- Use webpack to build UMD bundle (#22)
  - Build to `./dist/` directory before publish
- Fix regex bug on client parser (#24)
  - Add test for `window.DOMParser`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants