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

Add a rewrite callback to Editable.getContent() #238

Open
jenstroeger opened this issue Aug 27, 2021 · 6 comments
Open

Add a rewrite callback to Editable.getContent() #238

jenstroeger opened this issue Aug 27, 2021 · 6 comments

Comments

@jenstroeger
Copy link
Collaborator

jenstroeger commented Aug 27, 2021

First, I think the call to extractContent() in the Editable.getContent() function

return content.extractContent(element)

should actually be

return content.extractContent(element, false)

to ensure that the argument keepUiElements is a boolean as defined in the function’s documentation:

// @param {Boolean} Flag whether to keep ui elements like spellchecking highlights.

Technically, undefined is a falsey value, but it’s just not clean 🤓

Request

I’d like to modify/clean up the content of the host element before it’s serialized to HTML by passing in a callback function. Without it, getContent() returns a serialized HTML which I need to parse into a DOM element, clean it up, and serialize it again… which isn’t great.

So, for example:

const content = editable.getContent(elem, function(e) { 
    // Modify and clean up `e`  where `e` is the cloned and scrubbed `elem`.
});

If you’re interested in adding a callback function parameter to getContent() then I can open a PR. (It’s not a big change, but quite useful!)

@marcbachmann
Copy link
Member

We can also just add another method, which is more expressive than some callbacks.
For example return a DocumentFragment with editable.getContentFragment(elem).

@jenstroeger
Copy link
Collaborator Author

Oh, instead of returning the serialized HTML?

@jenstroeger
Copy link
Collaborator Author

@marcbachmann this is what you meant, correct? If you’re ok with the change then I can open a PR.

diff --git a/src/content.js b/src/content.js
index 6039b8e..3f00065 100644
--- a/src/content.js
+++ b/src/content.js
@@ -78,8 +78,8 @@ export function cleanInternals (element) {
 // @param {DOM node or document fragment} Element where to clean out the innerHTML.
 // If you pass a document fragment it will be empty after this call.
 // @param {Boolean} Flag whether to keep ui elements like spellchecking highlights.
-// @returns {String} The cleaned innerHTML of the passed element or document fragment.
-export function extractContent (element, keepUiElements) {
+// @returns {HTMLElement} The cleaned clone of the passed element or document fragment.
+export function extractContentFragment (element, keepUiElements) {
   const innerHtml = (element.nodeType === nodeType.documentFragmentNode
     ? getInnerHtmlOfFragment(element)
     : element.innerHTML
@@ -98,6 +98,19 @@ export function extractContent (element, keepUiElements) {
   // Remove line breaks at the end of a content block
   removeWhitespaces(clone, 'lastChild')
 
+  return clone
+}
+
+// Extracts the content from a host element. Does not touch or change the host.
+// Just returns the content fragment and removes elements marked for removal by
+// editable.
+//
+// @param {DOM node or document fragment} Element where to clean out the innerHTML.
+// If you pass a document fragment it will be empty after this call.
+// @param {Boolean} Flag whether to keep ui elements like spellchecking highlights.
+// @returns {String} The cleaned innerHTML of the passed element or document fragment.
+export function extractContent (element, keepUiElements) {
+  const clone = extractContentFragment(element, keepUiElements)
   return clone.innerHTML
 }
 
diff --git a/src/core.js b/src/core.js
index 54abda3..f40de6f 100644
--- a/src/core.js
+++ b/src/core.js
@@ -267,7 +267,19 @@ export class Editable {
    * @returns {String} The cleaned innerHTML.
    */
   getContent (element) {
-    return content.extractContent(element)
+    return content.extractContent(element, false)
+  }
+
+  /**
+   * Extract the content from an editable host or document fragment.
+   * This method will remove all internal elements and ui-elements.
+   *
+   * @param {DOM node or Document Fragment} The element or fragment whose cleaned up
+   * content fragment is returned.
+   * @returns {HTMLElement} The cloned and cleaned fragment.
+   */
+  getContentFragment (element)  {
+    return content.extractContentFragment(element, false)
   }
 
   /**

@marcbachmann
Copy link
Member

marcbachmann commented Sep 28, 2021

@peyerluk what do you think here? Return a div, which is easier to operate on or return a DocumentFragment, which supports easy insertion into the dom?

... Or just use the transform callback 😄

@jenstroeger
Copy link
Collaborator Author

I think working with a fragment is more in line with other code in this package (which works with fragments) and it avoids creating elements that have no other purpose than wrapping the fragment — in my use case I’d only unwrap the fragment again.

Either way, the boolean function parameter to content.extractContent() should be fixed.

@peyerluk
Copy link
Member

I would return a DocumentFragment. I think this is easy enough to work with.

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

No branches or pull requests

3 participants