Skip to content
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

Fix text node when pasting images; sanitize URLs #27

Merged
merged 8 commits into from
Aug 12, 2021

Conversation

oddtinker
Copy link
Contributor

@oddtinker oddtinker commented Aug 10, 2021

  • Assign an image's src as the parsed node's text content. Children must be passed to the jsx function as the third argument. Previously, they were returned as part of the attrs object.
image.mp4
  • Sanitize anchor's href, image's src

Resolves ENG-3787

@changeset-bot
Copy link

changeset-bot bot commented Aug 10, 2021

🦋 Changeset detected

Latest commit: 33d009e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@graphcms/html-to-slate-ast Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Aug 10, 2021

size-limit report 📦

Path Size
packages/react-renderer/dist/rich-text-react-renderer.cjs.production.min.js 4.82 KB (0%)
packages/react-renderer/dist/rich-text-react-renderer.esm.js 4.78 KB (0%)
packages/types/dist/rich-text-types.cjs.production.min.js 51 B (0%)
packages/types/dist/rich-text-types.esm.js 64 B (0%)

@oddtinker oddtinker changed the title Fix text node when pasting image node; sanitize URLs Fix text node when pasting images; sanitize URLs Aug 11, 2021
@oddtinker oddtinker marked this pull request as ready for review August 11, 2021 07:35
@linear
Copy link

linear bot commented Aug 11, 2021

Copy link
Contributor

@okjulian okjulian 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 the fix Larisa. Code looks spot on!

May I ask if it's possible to add a new test case that calls htmlToSlateAST with an html representation of the image you copied on the video, and verifies that the correct AST is generated? That test should have failed before your fix, and pass now.

Copy link
Contributor

@okjulian okjulian left a comment

Choose a reason for hiding this comment

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

Great fix, test and sanitizing addition 💯

@feychenie
Copy link
Contributor

:shipit:

@oddtinker oddtinker merged commit a594c49 into main Aug 12, 2021
@oddtinker oddtinker deleted the larisa/links-handling branch August 12, 2021 11:36
@github-actions github-actions bot mentioned this pull request Aug 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants