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

Dragging text to another mobiledoc-editor causes a Could not find parent section for un-mapped text node error #300

Closed
YoranBrondsema opened this issue Jan 22, 2016 · 10 comments · Fixed by #307
Assignees

Comments

@YoranBrondsema
Copy link
Collaborator

When I try to move text from one mobiledoc-editor to another, it throws an assertion error at this line: https://github.com/bustlelabs/mobiledoc-kit/blob/3a22d87869a439748b02379ed8bd49559cee7c50/src/js/utils/cursor/position.js#L148

I'm looking into it now but if you have an idea then let me know!

Yoran

@mixonic
Copy link
Contributor

mixonic commented Jan 22, 2016

@YoranBrondsema dragging is kind of messy. We need to replace the native functionality with our own cut/paste semantics.

@YoranBrondsema
Copy link
Collaborator Author

OK I understand. Do you have any sort of timeframe on this? Or an idea when this refactoring will be tackled?

It's that right now we have users that are losing work when dragging text from one editor to another. If it's going to take a long time before this will be added, I will look into a fix for cutting-pasting, or at least so that some cases are covered such as this one.

For this particular case (pasting text into an empty Mobiledoc) I think it can be solved by creating an empty p markup section before dumping the text into it. What do you think?

@mixonic
Copy link
Contributor

mixonic commented Jan 22, 2016

Ah, so this is very narrow error? Only when you drop into an empty document does it raise?

Drag/drop should work, it just doesn't work exactly like we want it to right now. For example, if you select and area with a card and drag/drop it the card itself is not duplicated.

However it should work as if you are pasting HTML. Even between mobiledoc-kit instances, and even into blank posts. If you want to take a look at it that would be great. I'm wary of adding an explicit default p, and there is some overlap between that and the placeholder feature, but even with the current DOM for a blank post things should function correctly.

@YoranBrondsema
Copy link
Collaborator Author

Yes it seems to be occurring only when it's an empty document.

Alright, we'll start working on a fix. Can't promise any dates but it should be soon as we want to get this fixed!

@mixonic
Copy link
Contributor

mixonic commented Jan 22, 2016

yeah please leave notes if you git it a shot, and if Cory or I pick it up we will be sure to mention it so there isn't any duplication of work. I feel the solution should be pretty easy to nail down.

@bantic
Copy link
Collaborator

bantic commented Jan 25, 2016

@YoranBrondsema I'm able to reproduce this. I'm working now on fixing the issue. Let me know if you have done any additional investigation or code and I'll make sure to incorporate it. Thanks!

@bantic bantic self-assigned this Jan 25, 2016
@YoranBrondsema
Copy link
Collaborator Author

@bantic I haven't looked at it this weekend so I didn't find out anything extra. I'd be happy to review any changes you make though!

@bantic
Copy link
Collaborator

bantic commented Jan 25, 2016

@YoranBrondsema Sounds good! Hopefully will have something for you tomorrow sometime

@bantic
Copy link
Collaborator

bantic commented Jan 27, 2016

@YoranBrondsema This led to a bigger refactoring of the mutation-observer/reparsing semantics in mobiledoc-kit, but I'm hoping to have something you can try out by end of day today.

@bantic
Copy link
Collaborator

bantic commented Jan 27, 2016

@YoranBrondsema this should be working now, if you want to try it out on the "better reparsing" branch

bantic added a commit that referenced this issue Jan 28, 2016
  * adds editor#_reparseSections and editor#_reparsePost
  * adds RenderTree#isDirty, use it to determine whether to rerender
    the cursor position after reparsing
  * tests for changing text and element nodes in the editor dom
  * Remove editor#reparse and #_reparseCurrentSection

fixes #300
bantic added a commit that referenced this issue Jan 28, 2016
  * adds editor#_reparseSections and editor#_reparsePost
  * adds RenderTree#isDirty, use it to determine whether to rerender
    the cursor position after reparsing
  * tests for changing text and element nodes in the editor dom
  * Remove editor#reparse and #_reparseCurrentSection

fixes #300
bantic added a commit that referenced this issue Jan 28, 2016
  * adds editor#_reparseSections and editor#_reparsePost
  * adds RenderTree#isDirty, use it to determine whether to rerender
    the cursor position after reparsing
  * tests for changing text and element nodes in the editor dom
  * Remove editor#reparse and #_reparseCurrentSection

fixes #300
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 a pull request may close this issue.

3 participants