getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.32k stars 168 forks source link

[v4] Custom marks always receive latest editor instance (not their own editor) #5456

Closed tobimori closed 1 year ago

tobimori commented 1 year ago

Description

See Video

This uses a custom mark (not tested with others) inside a Writer field, that's inside a group field which is used in a block fieldset. All fields have the same name and the same settings.

window.panel.plugin("sew-morlaix/extended", {
  writerMarks: {
    highlight: {
      get button() {
        return {
          icon: "palette",
          label: window.panel.$t("accent-color")
        }
      },

      commands() {
        return () => this.toggle()
      },

      get name() {
        return "highlight"
      },

      get schema() {
        return {
          parseDOM: [{ tag: "mark" }],
          toDOM: () => ["mark", 0]
        }
      }
    }
  }
})

Your setup

Kirby Version 4.0.0-alpha.6

distantnative commented 1 year ago

@tobimori could you please provide the blueprint too. my head already starts hurting trying to parse the hierarchy of nesting there 🤣

tobimori commented 1 year ago

@tobimori could you please provide the blueprint too. my head already starts hurting trying to parse the hierarchy of nesting there 🤣

It's a bit complex to share (because a lot of different block types) - broken down it looks like this:

distantnative commented 1 year ago

What does Group Heading + Toggle exactly refer to?

tobimori commented 1 year ago

What does Group Heading + Toggle exactly refer to?

A type: group; fields snippet so I can include more than one field. I don’t think it's relevant but I included it just in case.

distantnative commented 1 year ago

Hmm, can't even get your custom mark to work. Trying to replicate this is really hard with this little info. Please provide the full plugin incl styling for the highlight mark and the blueprint.

distantnative commented 1 year ago

Replication

Tobi shared the repo with me which helped me to create this minimal reproducible case:

Plugin

<?php

use Kirby\Cms\App;
use Kirby\Sane\Html;

// Custom allowed tags so they don't get stripped out by the sanitizer
Html::$allowedTags['mark'] = true;

App::plugin('sew-morlaix/extended', []);
window.panel.plugin("sew-morlaix/extended", {
    writerMarks: {
        highlight: {
            get button() {
                return {
                    icon: "palette",
                    label: window.panel.$t("accent-color"),
                };
            },

            commands() {
                return () => this.toggle();
            },

            get name() {
                return "highlight";
            },

            get schema() {
                return {
                    parseDOM: [{ tag: "mark" }],
                    toDOM: () => ["mark", 0],
                };
            },
        },
    },
});
.ProseMirror mark {
    color: gold;
    background: none;
    font-weight: bold;
}

Blueprint

layout:
  type: layout
  fieldsets:
    a:
      wysiwyg: true
      preview: fields
      fields:
        text:
          type: writer
          nodes: false
          marks:
            - highlight
    b:
      wysiwyg: true
      preview: fields
      fields:
        text:
          type: writer
          nodes: false
          marks:
            - highlight

Steps

  1. Add one layout row
  2. Add a block A
  3. Try out highlight mark, works
  4. Add a block B in the same layout row
  5. Highlight mark functionality either broken or triggers change in block B when editing block A
distantnative commented 1 year ago

Don't know if this is the origin for the issue, but https://github.com/getkirby/kirby/blob/v4/main/panel/src/components/Forms/Writer/Writer.vue#L247 doesn't just generate a new child class instance, but modified window.panel.plugins.writerMarks which should remain the plain plugin object. Haven't found a good way around it though.

distantnative commented 1 year ago

@tobimori is it fixed for you using these kirby/panel/dist files? dist.zip (make sure to clear the media folder too)

tobimori commented 1 year ago

yup - works!