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

Tiptap version 2 #6043

Merged
merged 46 commits into from
Jun 22, 2022
Merged

Tiptap version 2 #6043

merged 46 commits into from
Jun 22, 2022

Conversation

wiebkevogel
Copy link
Contributor

@wiebkevogel wiebkevogel commented May 16, 2022

This PR brings an update from Tiptap version 1 to 2 and replaces prosemirror-to-html and html-to-prosemirror with tiptap-php.

Tiptap Github:
https://github.com/ueberdosis/tiptap

Tiptap-PHP Github:
https://github.com/ueberdosis/tiptap-php

Documentation:
https://tiptap.dev/

Important: This PR is a breaking change because the integration of Marks, Nodes and Extensions no longer works as before due to the new extension API.

Packages

For this update, we had to load some tiptap extensions. We could have also used the StarterKit to map the main functionality, but this makes it harder to customize and extend via the bard fieldtype.

Menus

Tiptap no longer supports the component EditorMenuBar, but this is not dramatic because we can still get the desired functionality. 'BubbleMenu' and 'FloatingMenu' have also changed somewhat. It is now possible to set :tippy-options, so we could remove some custom styling.
Checking if a button is active and triggering a command has also changed a bit. Don't be suprised that we have extended the "heading-buttons" with the information activeName, because we couldn't work with the names H1, H2, etc. to check if a button is active.

Extensions

With the new extension API, replacing and extending existing extensions has changed.

New Extensions

Classes no longer need to be wrapped by helper functions (mark or node) anymore. An example of adding a new extension:

Statamic.$bard.addExtension(({ bard }) => {
    return MyExtension.configure({ bard });
});

The new extension should look like a Tiptap extension. Here is a detailed guide to Custom Extensions in Tiptap: https://tiptap.dev/guide/custom-extensions

Replacing Existing Extensions:

It is still possible to replace existing extensions:

Statamic.$bard.replaceExtension('heading', () => {
    return CustomHeading;
});

The difference is that we have to load the extension to be extended once in the custom extension:

import Heading from '@tiptap/extension-heading';

export const CustomHeading = Heading.extend({
    renderHTML({ node }) {
        const level = node.attrs.level;

        return [`h${level}`, { class: 'font-bold' }, 0];
    }
})

Buttons:

When adding a button to the toolbar, only the command changed:

Statamic.$components.register('CustomExtensionButton', require('./components/bard/CustomExtensionButton.vue').default);
Statamic.$bard.buttons(() => ({
    name: 'custom_extension',
    text: __('Custom Extension'),
    icon: 'icon',
    component: 'CustomExtensionButton',
    command: (editor) => editor.commands.setCustomExtension()
}));

TipTap API:

We have removed the Tiptap API from Bard.js for now, because we are not sure if we need to support this any longer and to what extent it would be possible at all.

PHP:

Due to the update to tiptap-php we could not use the renderer methods anymore. Therefore we have extended the Bard/Augmentor with the following functionalities:

public function renderHtmlToProsemirror(string $value)
{
    return (new Editor(['extensions' => $this->extensions()]))->setContent($value)->getDocument();
}

public function renderProsemirrorToHtml(array $value)
{
    return (new Editor(['extensions' => $this->extensions()]))->setContent($value)->getHTML();
}

Augmentor Class

The Augmentor class did change a lot. Those are the reasons why this feels right the right way:

  1. No need to differentiate between Marks and Nodes anymore if adding extensions.

  2. It felt that adding or replacing extensions could be improved.

  3. There has been two places where Extensions have beend added (BardServiceProvider and Augmentor)

Adding Extensions

This could be simplified a lot: https://github.com/statamic/cms/pull/6043/files#diff-6d5e2dfe3fb4feb16f6e7c84cf412b96fd872e8519c542987e8a0cd8b32af0e3L130-R118

// before

Augmentor::addNode($yourNode)

Augmentor::addMark($yourMark)



// now

Augmentor::addExtension('node-name', $yourNode)

Augmentor::addExtension('mark-name', $yourMark)

// even better

Augmentor::addExtensions([

'node-name' => $yourNode,

'mark-name' => $yourMark,

])

Replacing extensions

You needed the exact classname to make this possible. Passing a class itself did throw an error.

// before

Augmentor::replaceNode(\ProseMirrorToHtml\Nodes\Heading::class, $customNode)

Augmentor::replaceMark(\ProseMirrorToHtml\Marks\Foo::class, $yourMark)



// now

Augmentor::replaceExtension('heading', $yourNode)

Augmentor::replaceExtension('foo', $yourMark)

Extensions will be defined in the Service Provider now

As a lot of things did change, this felt like a nice clean up.

https://github.com/statamic/cms/pull/6043/files#diff-e29ce218f1b02291d06fd50ce74de831c947f858f8711d56f09c1e9fd2dc5acdR12-R36

Addon developers can easily look up which extensions are enabled by default and what their names are.

Tests:

We had to adjust the tests a bit, as the renderer method seems to work a little different and tiptap-php removes null values.

As PHP 8 is required, we removed PHP 7.4 from the testing matrix.

Bard Extensions

We are willing to help update Bard extensions. We did find the following extensions so far. Please let us know if we did forgett an addon.

Compatable with tiptap 2

Only working with tiptap 1

Feedback

For now, that should be all. We are curious what you think of the update. We have created a repo that can be used to test this changes: https://github.com/spiegeltechlab/statamic-tiptap2-test-repo

@jackmcdade
Copy link
Member

Giving this a good test right now and it's looking fantastic! The only thing I've come across so far is a weird glitch when dragging sets. Here's a quick video of what I'm seeing:

CleanShot.2022-05-16.at.13.23.55.mp4

Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

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

I've found an issue. We'll need to handle the extensions that are now renamed in Tiptap 2.

This bit:

CleanShot 2022-06-01 at 10 25 06

To see the issue, make a list using tiptap 1. It will be saved like this.

bard_field:
  -
    type: paragraph
    content:
      -
        type: text
        text: 'this is a list:'
  -
    type: bullet_list
    content:
      -
        type: list_item
        content:
          -
            type: paragraph
            content:
              -
                type: text
                text: one
      -
        type: list_item
        content:
          -
            type: paragraph
            content:
              -
                type: text
                text: two
  -
    type: paragraph
    content:
      -
        type: text
        text: 'that was a list.'

Then if you swap to tiptap 2, it won't recognize bullet_list or list_item. You'll need to rename them to bulletList and listItem.

In the CP, the bard fieldtype will be blank and have console errors.
On the frontend, it outputs incorrect html.

We could technically create an upgrade script that would adjust all the bard fields in all the content, but that feels very heavy handed and error prone.

Maybe we could somehow work out ways to alias the older snake_case names on the fly. This would need to happen before it gets to the javascript, and during augmentation

Before you dive in and fix this, we should discuss ways to address this.

@jasonvarga
Copy link
Member

I've asked on the tiptap discussion board whether they'd be open to allowing aliases.
ueberdosis/tiptap#2839

@jackmcdade
Copy link
Member

So aliases are a no go, as per the Uberdosis team (Jason spoke with them off Github), but they proposed a name overwrite as decent alternative.

const CustomBulletList = BulletList.extend({ name: 'bulletList' }}

We can tackle this in a separate PR, so we'll go ahead and get this merged in.

@jackmcdade jackmcdade dismissed jasonvarga’s stale review June 21, 2022 00:47

Aliasing isn't possible, we'll have to tackle this with an overwrite instead. Can do this separate from this PR.

@jackmcdade
Copy link
Member

We have some merge conflicts here, once those are sorted we can merge.

# Conflicts:
#	package-lock.json
#	resources/js/components/fieldtypes/bard/Doc.js
#	resources/js/components/fieldtypes/bard/Set.js
#	resources/js/components/fieldtypes/bard/Set.vue
@wiebkevogel
Copy link
Contributor Author

merge conflicts are solved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants