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

Copy Paste #147

Merged
merged 1 commit into from
Oct 22, 2015
Merged

Copy Paste #147

merged 1 commit into from
Oct 22, 2015

Conversation

bantic
Copy link
Collaborator

@bantic bantic commented Sep 17, 2015

ready to merge

This adds support for copy-pasting content from one Content Kit to another, and from Google Docs to Content-Kit.

  • Rename PostParser -> DOMParser, remove old DOMParser
  • Add HTMLParser (parses from HTML to PostAbstract)
  • Add many fixtures with example HTML from copying in a google doc document
  • Add postEditor#insertPost. It inserts the sections from a post into the current editor's post at the passed Position, with some logic for merging/inserting into the section with the cursor in it. It returns a Position used to position the cursor after inserting content
  • Only allow certain attributes from parsed content (currently only "href" and "rel" are acceptable attributes. These are defined in Markup's VALID_ATTRIBUTES)
  • support for pasting multi-section copy data
  • support for pasting unstructured HTML
  • lock down allowed element attributes when parsing HTML

For a separate PR:

  • support for copying non-markerable sections like cards (and list sections)
  • parse lists properly from HTML. They currently cause a MarkupSection with tagName "li" to be created, but we want them to cause a ListSection and ListItem(s) to be created instead. (parse pasted HTML that includes LI items properly #183)

For later:

  • Add the ability to handle more exotic pasted data, such as images.

fixes #111

@bantic
Copy link
Collaborator Author

bantic commented Oct 21, 2015

Many tests added for parsing google-docs-style copy/pasted HTML

@@ -6,10 +6,17 @@ import {
VALID_MARKUP_SECTION_TAGNAMES
} from 'content-kit-editor/models/markup-section';

function popMany(arr, count) {
Copy link
Contributor

Choose a reason for hiding this comment

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

seems like good use for splice. These should be the same:

popMany(array, 5);
array.splice(-5, 5, []);

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍 it's actually array.splice(-length, +length) (no []), but yeah. Changing this. I did that because I can never remember the arguments to splice

@mixonic
Copy link
Contributor

mixonic commented Oct 22, 2015

Lookin' pretty sweet @bantic :-) :-) :-) :-) :-)

@bantic bantic force-pushed the copy-paste-111 branch 9 times, most recently from 4b8be3b to d76ffe1 Compare October 22, 2015 17:16
@bantic
Copy link
Collaborator Author

bantic commented Oct 22, 2015

@mixonic Ready for 👓 when you can.

copypaste

@bantic bantic changed the title WIP Adds tests for copy and pasting markers in a single section Copy Paste Oct 22, 2015
@bantic
Copy link
Collaborator Author

bantic commented Oct 22, 2015

If you're curious to see what the HTML result of copy/pasting something from Google docs looks like, you can use this tool to see it.

const mobiledoc = this.post.cloneRange(range);
const html = `<div data-mobiledoc='${JSON.stringify(mobiledoc)}'></div>`;
clipboardData.setData('text/html', html);
event.preventDefault();
Copy link
Contributor

Choose a reason for hiding this comment

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

@bantic this logic means we cannot meaningfully copy out of Content-Kit to third parties?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah.
The preventDefault is necessary for the text/html that we set on the above line to be preserved.
Rendering the HTML with the HTML Renderer and inserting it in the div above would preserve copy-to-outside-world functionality.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Or, honestly, just letting natural browser copy do its thing for now would work prety well. We'll reparse the HTML on the way back in rather than using the already-serialized mobiledoc, but that doesn't seem to be a problem. Eventually we will want to keep that serialized mobiledoc so that cards can be preserved and copy/pasted around.

Copy link
Contributor

Choose a reason for hiding this comment

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

Aw man that would be cool. Opened a followup #180

Copy link
Contributor

Choose a reason for hiding this comment

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

Or, honestly, just letting natural browser copy do its thing for now would work prety well. We'll reparse the HTML on the way back in rather than using the already-serialized mobiledoc, but that doesn't seem to be a problem. Eventually we will want to keep that serialized mobiledoc so that cards can be preserved and copy/pasted around.

Ah, per another comment here I was surprised not to see cards being copied. I think both these things need followups if they are not in scope here. Copying mobiledocs cards should be pretty trivial.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cool, after this merges I'll add a follow-up PR to address #180.

@bantic
Copy link
Collaborator Author

bantic commented Oct 22, 2015

needs to handle "cut" in addition to "copy" event

  * Rename PostParser -> DOMParser, remove old DOMParser
  * Add many fixtures with example HTML from copying in a google doc document
  * Add postEditor#insertPost. It returns a Position used to position the cursor after inserting content
  * Only allow certain attributes from parsed content
  * filter markup attributes at instantiation
  * when copying, override the native HTML and text copied to display a link to copy-paste issue #180
  * copy/paste cards and lists
  * Handle cut event

fixes #111
bantic added a commit that referenced this pull request Oct 22, 2015
@bantic bantic merged commit 5f2dcfc into master Oct 22, 2015
@bantic bantic deleted the copy-paste-111 branch October 22, 2015 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle copy/pasting properly
2 participants