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

Improve paste handling (more cases, more tests) #2436

Merged
merged 5 commits into from Aug 16, 2017

Conversation

Projects
None yet
3 participants
@iseulde
Member

iseulde commented Aug 16, 2017

  • This PR adds more handling to blocks: PRE, PRE/CODE, TABLE, HR and BLOCKQUOTE.
  • Reorganises the files a bit.
  • It adds test for the filters.
  • Fixes case where nested spans are not stripped.
  • It improves non-semantic tag stripping (remove DIV and SPAN). This ensures that nested tags are handled by blocks instead of freeform. E.g.:
<div>
  <pre>...</pre>
</div>

@iseulde iseulde self-assigned this Aug 16, 2017

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Aug 16, 2017

Codecov Report

Merging #2436 into master will increase coverage by 0.13%.
The diff coverage is 82.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2436      +/-   ##
==========================================
+ Coverage   26.44%   26.57%   +0.13%     
==========================================
  Files         157      158       +1     
  Lines        4851     4876      +25     
  Branches      819      823       +4     
==========================================
+ Hits         1283     1296      +13     
- Misses       3015     3024       +9     
- Partials      553      556       +3
Impacted Files Coverage Δ
blocks/api/paste/normalise-blocks.js 96% <ø> (ø)
blocks/library/preformatted/index.js 40% <0%> (-4.45%) ⬇️
blocks/library/code/index.js 42.85% <0%> (-7.15%) ⬇️
blocks/library/table/index.js 33.33% <0%> (-3.04%) ⬇️
blocks/library/separator/index.js 40% <0%> (-10%) ⬇️
blocks/api/paste/strip-wrappers.js 100% <100%> (ø)
blocks/api/paste/index.js 100% <100%> (ø)
blocks/library/quote/index.js 13.88% <50%> (-0.4%) ⬇️
blocks/library/cover-text/index.js 35% <0%> (ø) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c8c8078...667fd23. Read the comment docs.

codecov bot commented Aug 16, 2017

Codecov Report

Merging #2436 into master will increase coverage by 0.13%.
The diff coverage is 82.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2436      +/-   ##
==========================================
+ Coverage   26.44%   26.57%   +0.13%     
==========================================
  Files         157      158       +1     
  Lines        4851     4876      +25     
  Branches      819      823       +4     
==========================================
+ Hits         1283     1296      +13     
- Misses       3015     3024       +9     
- Partials      553      556       +3
Impacted Files Coverage Δ
blocks/api/paste/normalise-blocks.js 96% <ø> (ø)
blocks/library/preformatted/index.js 40% <0%> (-4.45%) ⬇️
blocks/library/code/index.js 42.85% <0%> (-7.15%) ⬇️
blocks/library/table/index.js 33.33% <0%> (-3.04%) ⬇️
blocks/library/separator/index.js 40% <0%> (-10%) ⬇️
blocks/api/paste/strip-wrappers.js 100% <100%> (ø)
blocks/api/paste/index.js 100% <100%> (ø)
blocks/library/quote/index.js 13.88% <50%> (-0.4%) ⬇️
blocks/library/cover-text/index.js 35% <0%> (ø) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c8c8078...667fd23. Read the comment docs.

@iseulde iseulde requested a review from georgeh Aug 16, 2017

@iseulde iseulde requested a review from aduth Aug 16, 2017

@georgeh

👍 I like seeing how isMatch() can push the paste logic into the individual blocks.

@iseulde

This comment has been minimized.

Show comment
Hide comment
@iseulde

iseulde Aug 16, 2017

Member

Yeah, though I'd still prefer if we tried to auto-match. Maybe in the future :)

Member

iseulde commented Aug 16, 2017

Yeah, though I'd still prefer if we tried to auto-match. Maybe in the future :)

@iseulde iseulde merged commit 8274602 into master Aug 16, 2017

3 checks passed

codecov/project 26.57% (+0.13%) compared to c8c8078
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@iseulde iseulde deleted the try/more-paste-handling branch Aug 16, 2017

type: 'raw',
isMatch: ( node ) => (
node.nodeName === 'PRE' &&
node.children === 1 &&

This comment has been minimized.

@aduth

aduth Aug 30, 2017

Member

Since .children is the NodeList, should this be testing .children.length or .childElementCount ? A few other occurrences of this in the pull request.

@aduth

aduth Aug 30, 2017

Member

Since .children is the NodeList, should this be testing .children.length or .childElementCount ? A few other occurrences of this in the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment