Skip to content
This repository has been archived by the owner on Feb 6, 2023. It is now read-only.

Make default text content of img blocks a little camera emoji #1378

Closed
wants to merge 3 commits into from

Conversation

flarnie
Copy link
Contributor

@flarnie flarnie commented Sep 10, 2017

what is the change?:
We don't allow pasting whitespace-only blocks - see #231

In the past we set the 'src' as the text content of an 'img' block when
pasting/converting HTML, but sometimes the 'src' is really long and can
even crash the editor. Plus that's not really what users expect when
they paste an image.

An empty string as a default would be better but the code in
convertFromHTML needs refactored and for now I don't want to touch it.
The camera emoji is a cute and easy fix.

why make this change?:
Right now images are being dropped by `convertFromHTML

test plan:
Manual testing with examples/draft-0-10-0/convertFromHTML/convert.html

We should do a follow-up change to add a test that catches this.

issue:
#1377

Before submitting a pull request, please make sure the following is done...

  1. Fork the repo and create your branch from master.
  2. If you've added code that should be tested, add tests!
  3. If you've changed APIs, update the documentation.
  4. Ensure that:
  • The test suite passes (npm test)
  • Your code lints (npm run lint) and passes Flow (npm run flow)
  • You have followed the testing guidelines
  1. If you haven't already, complete the CLA.

Please use the simple form below as a guideline for describing your pull request.

Thanks for contributing to Draft.js!

Summary

[...]

Test Plan

[...]

**what is the change?:**
We don't allow pasting whitespace-only blocks - see facebookarchive#231

In the past we set the 'src' as the text content of an 'img' block when
pasting/converting HTML, but sometimes the 'src' is really long and can
even crash the editor. Plus that's not really what users expect when
they paste an image.

An empty string as a default would be better but the code in
`convertFromHTML` needs refactored and for now I don't want to touch it.
The camera emoji is a cute and easy fix.

**why make this change?:**
Right now images are being dropped by `convertFromHTML

**test plan:**
Manual testing with examples/draft-0-10-0/convertFromHTML/convert.html

We should do a follow-up change to add a test that catches this.

**issue:**
facebookarchive#1377
@flarnie
Copy link
Contributor Author

flarnie commented Sep 10, 2017

For future reference - here is where the space-only blocks are dropped:

https://github.com/facebook/draft-js/blob/master/src/model/encoding/convertFromHTMLToContentBlocks.js#L285-L296

@aadsm
Copy link
Contributor

aadsm commented Sep 10, 2017

I love the camera emoji replacement!

@flarnie flarnie changed the title Make default text content of img blocks a little camera emoji 📷 Make default text content of img blocks a little camera emoji Sep 11, 2017
@facebook-github-bot
Copy link

@flarnie has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

2 similar comments
@facebook-github-bot
Copy link

@flarnie has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link

@flarnie has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit that referenced this pull request Sep 10, 2018
Summary:
Since draft.js replaces pasted images as a camera emoji to indicate pasted content, text copied as:

{F136101602}

which is copied to clipboard as
```
"<meta charset='utf-8'><span style="color: rgb(29, 33, 41); font-family: system-ui, -apple-system, system-ui, &quot;.SFNSText-Regular&quot;, sans-serif; font-size: 24px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 300; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255); text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;">hello<span> </span></span><span class="_5mfr _47e3" style="line-height: 0; vertical-align: middle; margin: 0px 1px; font-family: system-ui, -apple-system, system-ui, &quot;.SFNSText-Regular&quot;, sans-serif; color: rgb(29, 33, 41); font-size: 24px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 300; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255); text-decoration-style: initial; text-decoration-color: initial;"><img class="img" height="24" role="presentation" src="https://static.xx.fbcdn.net/images/emoji.php/v9/f4a/2/24/1f600.png" width="24" alt="" style="border: 0px; vertical-align: -3px; margin-top: 0px;"><span class="_7oe" style="display: inline; font-size: 0px; width: 0px; margin-bottom: 0px; font-family: inherit;">?</span></span>"
```
essentially `<span><img /><span>...</span></span>`

when pasted, renders as:
{F136101606}

This behavior is not required when pasting images that are for presentation only, so adding the camera emoji is skipped for this use case. This is an update to the functionality added in #1378

Rolling this out with gatekeeper `draftjs_fix_paste_for_img`

Differential Revision: D9333247

fbshipit-source-id: 772229521b4457ea2a6efd726715710c03c4887d
jdecked pushed a commit to twitter-forks/draft-js that referenced this pull request Oct 9, 2019
Summary:
Since draft.js replaces pasted images as a camera emoji to indicate pasted content, text copied as:

{F136101602}

which is copied to clipboard as
```
"<meta charset='utf-8'><span style="color: rgb(29, 33, 41); font-family: system-ui, -apple-system, system-ui, &quot;.SFNSText-Regular&quot;, sans-serif; font-size: 24px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 300; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255); text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;">hello<span> </span></span><span class="_5mfr _47e3" style="line-height: 0; vertical-align: middle; margin: 0px 1px; font-family: system-ui, -apple-system, system-ui, &quot;.SFNSText-Regular&quot;, sans-serif; color: rgb(29, 33, 41); font-size: 24px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 300; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255); text-decoration-style: initial; text-decoration-color: initial;"><img class="img" height="24" role="presentation" src="https://static.xx.fbcdn.net/images/emoji.php/v9/f4a/2/24/1f600.png" width="24" alt="" style="border: 0px; vertical-align: -3px; margin-top: 0px;"><span class="_7oe" style="display: inline; font-size: 0px; width: 0px; margin-bottom: 0px; font-family: inherit;">?</span></span>"
```
essentially `<span><img /><span>...</span></span>`

when pasted, renders as:
{F136101606}

This behavior is not required when pasting images that are for presentation only, so adding the camera emoji is skipped for this use case. This is an update to the functionality added in facebookarchive#1378

Rolling this out with gatekeeper `draftjs_fix_paste_for_img`

Differential Revision: D9333247

fbshipit-source-id: 772229521b4457ea2a6efd726715710c03c4887d
midas19910709 added a commit to midas19910709/draft-js that referenced this pull request Mar 30, 2022
Summary:
**what is the change?:**
We don't allow pasting whitespace-only blocks - see facebookarchive/draft-js#231

In the past we set the 'src' as the text content of an 'img' block when
pasting/converting HTML, but sometimes the 'src' is really long and can
even crash the editor. Plus that's not really what users expect when
they paste an image.

An empty string as a default would be better but the code in
`convertFromHTML` needs refactored and for now I don't want to touch it.
The camera emoji is a cute and easy fix.

**why make this change?:**
Right now images are being dropped by `convertFromHTML

**test plan:**
Manual testing with examples/draft-0-10-0/convertFromHTML/convert.html

We should do a follow-up change to add a test that catches this.

**issue:**
facebookarchive/draft-js#1377

*Before* submitting a pull request, please make sure the following is done...

1. Fork the repo and create your branch from `master`.
2. If you've added code that should be tested, add tests!
3. If you've changed APIs, update the documentation.
4. Ensure that:
  * The test suite passes (`npm test`)
  * Your code lints (`npm run lint`) and passes Flow (`npm run flow`)
  * You have followed the [testing guidelines](https://github.com/facebook/draft-js/wiki/Testing-for-Pull-Requests)
5. If you haven't already, complete the [CLA](https://code.facebook.com/cla).

Please use the simple form below as a guideline for describing your pull request.

Thanks for contributing to Draft.js!

-

**Summary**

[...]

**Test Plan**

[...]
Closes facebookarchive/draft-js#1378

Differential Revision: D5804382

fbshipit-source-id: f465a1a92155b02c5650acb9622e27e4e3714492
alicayan008 pushed a commit to alicayan008/draft-js that referenced this pull request Jul 4, 2023
Summary:
**what is the change?:**
We don't allow pasting whitespace-only blocks - see facebookarchive/draft-js#231

In the past we set the 'src' as the text content of an 'img' block when
pasting/converting HTML, but sometimes the 'src' is really long and can
even crash the editor. Plus that's not really what users expect when
they paste an image.

An empty string as a default would be better but the code in
`convertFromHTML` needs refactored and for now I don't want to touch it.
The camera emoji is a cute and easy fix.

**why make this change?:**
Right now images are being dropped by `convertFromHTML

**test plan:**
Manual testing with examples/draft-0-10-0/convertFromHTML/convert.html

We should do a follow-up change to add a test that catches this.

**issue:**
facebookarchive/draft-js#1377

*Before* submitting a pull request, please make sure the following is done...

1. Fork the repo and create your branch from `master`.
2. If you've added code that should be tested, add tests!
3. If you've changed APIs, update the documentation.
4. Ensure that:
  * The test suite passes (`npm test`)
  * Your code lints (`npm run lint`) and passes Flow (`npm run flow`)
  * You have followed the [testing guidelines](https://github.com/facebook/draft-js/wiki/Testing-for-Pull-Requests)
5. If you haven't already, complete the [CLA](https://code.facebook.com/cla).

Please use the simple form below as a guideline for describing your pull request.

Thanks for contributing to Draft.js!

-

**Summary**

[...]

**Test Plan**

[...]
Closes facebookarchive/draft-js#1378

Differential Revision: D5804382

fbshipit-source-id: f465a1a92155b02c5650acb9622e27e4e3714492
aforismesen added a commit to aforismesen/draft-js that referenced this pull request Jul 12, 2024
Summary:
**what is the change?:**
We don't allow pasting whitespace-only blocks - see facebookarchive/draft-js#231

In the past we set the 'src' as the text content of an 'img' block when
pasting/converting HTML, but sometimes the 'src' is really long and can
even crash the editor. Plus that's not really what users expect when
they paste an image.

An empty string as a default would be better but the code in
`convertFromHTML` needs refactored and for now I don't want to touch it.
The camera emoji is a cute and easy fix.

**why make this change?:**
Right now images are being dropped by `convertFromHTML

**test plan:**
Manual testing with examples/draft-0-10-0/convertFromHTML/convert.html

We should do a follow-up change to add a test that catches this.

**issue:**
facebookarchive/draft-js#1377

*Before* submitting a pull request, please make sure the following is done...

1. Fork the repo and create your branch from `master`.
2. If you've added code that should be tested, add tests!
3. If you've changed APIs, update the documentation.
4. Ensure that:
  * The test suite passes (`npm test`)
  * Your code lints (`npm run lint`) and passes Flow (`npm run flow`)
  * You have followed the [testing guidelines](https://github.com/facebook/draft-js/wiki/Testing-for-Pull-Requests)
5. If you haven't already, complete the [CLA](https://code.facebook.com/cla).

Please use the simple form below as a guideline for describing your pull request.

Thanks for contributing to Draft.js!

-

**Summary**

[...]

**Test Plan**

[...]
Closes facebookarchive/draft-js#1378

Differential Revision: D5804382

fbshipit-source-id: f465a1a92155b02c5650acb9622e27e4e3714492
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants