ueberdosis / tiptap

The headless rich text editor framework for web artisans.
https://tiptap.dev
MIT License
27.56k stars 2.29k forks source link

Simplify `parseHTML` method of attributes to return the value itself #1863

Closed philippkuehn closed 3 years ago

philippkuehn commented 3 years ago

The problem I am facing At the moment it’s kind of repetitive to return an object within parseHTML.

addAttributes() {
  return {
    id: {
      default: null,
      parseHTML: element => {
        return {
          id: element.getAttribute('id'),
        }
      },
      renderHTML: attributes => {
        return {
          id: attributes.id,
        }
      },
    },
  }
}

The solution I would like It would be great to just return the value because it’s already clear that it’s all about the attribute itself.

addAttributes() {
  return {
    id: {
      default: null,
      parseHTML: element => element.getAttribute('id'),
      renderHTML: attributes => {
        return {
          id: attributes.id,
        }
      },
    },
  }
}

Additional context The initial idea was to be able to set multiple attributes within parseHTML but this some kind of unexpected and wasn’t needed until now so I think we can safely remove that feature.

But anyway, this would be a breaking change but I don't think it makes sense to build a fallback to the old behavior.

hanspagel commented 3 years ago

I think that’s fine. 👍️

This breaking change would probably lead to issues that are not visible at the first glance. I think we should at least add a console.warn when an object is returned.

tylerforesthauser commented 3 years ago

Ah crud, I actually have a use-case for returning an object. I've got one attribute that influences another, mostly for our own backwards compatibility. So if certain conditions are met, I'm returning the values for both attributes in the single attribute's parseHTML method. With this change, is that still possible?

philippkuehn commented 3 years ago

Oh no, this is not possible right now. And I’m not sure if we could support both. It becomes difficult to differ between these two cases because you can also save objects as attributes.

hanspagel commented 3 years ago

Can’t you just add a parseHTML function for both attributes and do (nearly) the same in both functions? That should be an okayish workaround, or am I wrong?

const CustomParagraph = Paragraph.extend({
  addAttributes() {
    return {
      color: {
        parseHTML: element => element.getAttribute('data-color'),
      },
      legacyColor: {
        parseHTML: element => element.getAttribute('data-color'),
      },
    }
  },
})
igor-ye commented 2 years ago

Hope you will not remove this feature in future versions. We also use objects stored in single attribute for custom Vue components properties. So we have only one custom node (componentNode) rendered as <component> tag with only two attributes - is and v-bind. And only one wrapping ComponentNodeEditor.vue for choosing editor type from the list:

<template>
    <node-view-wrapper class="component-editor">
        <a-select v-if="!componentName" :value="componentName" @change="componentSelected" placeholder="Select a block">
            <a-select-option v-for="e in blockEditors" :value="e">{{ e }}</a-select-option>
        </a-select>
        <component v-else :is="componentName" class="instance" :bindings="componentBindings" :updateProps="updateProps" />
    </node-view-wrapper>
</template>

So you don't need to write repetitive code in Node.create for every new component editor. Just one new MyNewCustomNodeEditor.vue which saves all props at once passing an object to updateProps.