-
Notifications
You must be signed in to change notification settings - Fork 194
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
h-elements, lists, images, other block elements #48
Comments
If you play around with Microsoft Word's track changes, they merge blocks, with the difference that they keep track of block boundaries so you can reject/restore changes. If it is overly problematic to retain block merging, then I would consider getting rid of it. |
It's not impossible to keep it the way you have it right now. Right now it's there as an option that can be turned on or off. I don't know your guys'es usecase that well. It would seem to me that if you do merge blocks, you'd want to keep track of the block boundaries so you can restore them in case you don't like the changes at all (the way you describe MsWord). Is that not a concern? |
Merging was a preference since that is the way MsWord handles when deleting from block to block. Also, if blocks aren't merged, then it forces the implementer to remove empty blocks when cleaning since they will be emptied by ice during cleaning. It's just a preference and I would be willing to remove it, especially if it introduces a lot of work when we support more blocks. |
Ok, I have added simple tests for deleting images and lists from the left and right and made it all work in Firefox and Webkit. Further tests should be made whenever one comes across a problem, I would think. I have also removed the mergeBlocks option. The changes overall are in a few extra functions I have added to dom.js, and in ICE a rewrite of _deleteFromLeft, _deleteFromRight, _deleteFromSelection as well as replacing the _addTextNodeTracking function with an _addNodeTracking function that can cover other things than text nodes. It can be used to move the range, but if you don't hand it a range, it can work with just marking the node for deletion as well. I initially pulled from some earlier trials of getting images and lists to work with ICE, and have then developed that further. I think it is a good idea to take it piecemeal, but in the case of the rewrite of these functions, one almost has to look at them in their entirety. I have tried to comment as much as possible. |
Closing this since we finished #49 where we decided to remove the block merging. |
I have been working on support for lists, and images. Integration tests will still have to be written (will do some of that).
Also, it should be figuired otu how this would work with merge blocks. I am not exactly sure what the point is with merging blocks, but it would seem to me that merging anything else than p-elements with one-another, would not make a lot of sense, right? So if you have a list following a paragraph and you delete it all, the list and paragraph should not merge into one, right?
The text was updated successfully, but these errors were encountered: