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

Should we add components instead of scoped slots? #20

Closed
philippkuehn opened this Issue Sep 10, 2018 · 15 comments

Comments

Projects
None yet
3 participants
@philippkuehn
Copy link
Contributor

philippkuehn commented Sep 10, 2018

Currently we're using scoped slots for content and menus.

<editor>
  <!-- Content goes here -->
  <div slot="content" slot-scope="props">
    <p>some text</p>
  </div>

  <!-- Render menubar -->
  <div slot="menubar" slot-scope="{ nodes }">
    <div v-if="nodes">
      <button @click="nodes.heading.command({ level: 1 })">
        H1
      </button>
    </div>
  </div>

  <!-- Render menububble -->
  <div slot="menububble" slot-scope="{ marks }">
    <div v-if="marks">
      <button @click="marks.bold.command()">
        Bold
      </button>
    </div>
  </div>
</editor>

There are two limitations for doing so:

  1. In the menububble slot we have to add a check for marks and nodes like this <div v-if="marks">. For one tick these values are null. It would be nice to render these slots only when these values are available but then we do not have access to the DOM element of this slot which is required for the menububble plugin.
  2. Nested markup is not supported.

Maybe it would be nice to use components like this:

<editor>
  <!-- Content goes here -->
  <editor-content>
    <p>some text</p>
  </editor-content>

  <!-- Render menubar -->
  <editor-menubar>
    <template slot-scope="{ nodes }">
      <button @click="nodes.heading.command({ level: 1 })">
        H1
      </button>
    </template>
  </editor-menubar>

  <!-- Render menububble -->
  <editor-menububble>
    <template slot-scope="{ marks }">
      <button @click="marks.bold.command()">
        Bold
      </button>
    </template>
  </editor-menububble>
</editor>

But I'm not sure how these components will work together and sync its editor state. With provide and inject or an event bus or passing props? 🤷‍♂️

@jaysaurus

This comment has been minimized.

Copy link

jaysaurus commented Sep 14, 2018

Yeah, I was mulling this over too. scoped slots make for big templates!!

Presumably, you could manage the state via vuex (the aforementioned event bus, effectively) on the module side. for the clients actually developing their editors, you could expose events like @init and @update.

@qyloxe

This comment has been minimized.

Copy link

qyloxe commented Sep 26, 2018

Well, maybe the architecture like in quasar framework WYSIWYG editor - it's quite thoughtful and adjusted to many use cases:

https://quasar-framework.org/components/editor---wysiwyg.html

BTW - I would LOVE if quasar editor would be replaced with tiptap editor...

@philippkuehn

This comment has been minimized.

Copy link
Contributor Author

philippkuehn commented Sep 26, 2018

@qyloxe Thanks for your input! I do not like to configure everything as long arrays via props. That‘s the way most of all editors are configurable and that‘s why I started creating tiptap 🙌 For me it‘s very important to have full control over markup because you can use text editors for so many different use cases today, that it differs to much.

@jaysaurus

This comment has been minimized.

Copy link

jaysaurus commented Sep 26, 2018

I'm sort of putting something together that's more in keeping with injected components using prose mirror. I'll give you a shout @qyloxe if I make something public. Failing that, it's not too bad getting something up and running just using prose mirror as is in vue. Likewise @philippkuehn, if you're interested. Not a dig at your hard work, i think tiptap is decent but I couldn't figure out how to get it to do quite what I wanted it to do and my use case needs something pretty complex!

@qyloxe

This comment has been minimized.

Copy link

qyloxe commented Sep 26, 2018

@philippkuehn OK. Those editors work this way because they want you to respond to specific events in very specific, preconfigured context. If you want to allow different architectures then assuming that user should only respond to events this is obviously wrong. I understand that you do not want only events (as a block for describing dynamic behaviour) but you want the whole architectural flexibility. And I do like it :-) I like it, because you are using ProseMirror which is designed with similar flexibility in mind and you also want to "be free". So, lets imagine the most difficult scenario and try to design the correct architecture to model it in the desired environment. I propose to design an interface for three-way merge editor with custom nodes, marks, extensions, synchronized with other collaborators, preserving versions and themable, mobile and accesible. Oh, and also print friendly and possible to configure without much programming knowledge (UI is separated from behaviour).

Let's start with something simple:

option 1 for three-way merge:

<pk-tiptap>
   <pk-editor role="base" />
   <pk-editor role="version-1"/>
   <pk-editor role="version-2"/>
   <pk-editor role="output" />
</pk-tiptap>

option 2 for three-way merge:

<pk-tiptap name="merge1" />
<pk-editor role="base" manager="merge1" />
<pk-editor role="version-1" manager="merge1" />
<pk-editor role="version-2" manager="merge1" />
<pk-editor role="output" manager="merge1" />

In option1 the hidden "manager" is deducted from components hierarchy. In option 2, there could be many components and every one needs to specify his own "manager" explicitly. What is "manager"? Well, in every case you will need something for event processing, context preserving, communication bus, serialization, default configuration etc. In your present architecture, you have tightly coupled presentation hierarchy and behaviour (everything is in "editor" component). What I propose is separation of "manager" and "presentation" and what is needed from you, is to decide - on the architectural level: do you prefer hierarchical coupling (option 1) or "loose" coupling (option 2)? Every next decision could be derived from that one.

@philippkuehn

This comment has been minimized.

Copy link
Contributor Author

philippkuehn commented Sep 26, 2018

@jaysaurus would love to hear what you trying to do! What wasn‘t possible with tiptap?

@philippkuehn

This comment has been minimized.

Copy link
Contributor Author

philippkuehn commented Sep 26, 2018

@qyloxe Thanks for this write-up! Basically I'm thinking of these two solutions. I'm a fan of simplicity so I tend to option 1 even if option 2 is more "powerful". I will soon create a roadmap for v1 of tiptap and collect some features to implement and this one is definitely on the list.

@qyloxe

This comment has been minimized.

Copy link

qyloxe commented Sep 26, 2018

@philippkuehn just $0.02 more :-) with option 1 you end up with rather closed design. Why? Because your component will be definitely used in one of the VUE frameworks: quasar, nuxt, vuetify, bulma or whatnot. When used in framework, with option 1 it is extremely hard to decompose your component into said framework architecture. With option 2 you could easily write something like this:

<!-- changed to REF - it could work like that in vue -->
<pk-tiptap ref="merge1" />

<q-tab>
 <q-card>
    <pk-editor role="base" manager="merge1" />
 </q-card>
</q-tab>
<q-tab>
 <q-card>
    <pk-editor role="version-1" manager="merge1" />
 </q-card>
</q-tab>
...  

so, option 2 is framework agnostic. In VUE it is rather easy to write with option 2 because you could use refs or private (in manager context) vue event bus or even vuex BUT using vuex is not architecturally clean, because there are people who use something else for state management.

in option 2 it is easier to extend your components because one does not have to worry where he should put his component in your component hierarchy but only concentrate on following your easy and simple interface. In option 1 one need to think about structure (which is painful) in option 2 one can think about interaction (which is easy because one do not have to know all the quirks of the structure). In effect you will have more contributions and greater library of subcomponents, plugins, mixins and directives from community, where in option 1 you will have to "approve" or "check" or "review" each and every single contribution. More work IMO.

OK, it was rather $0.06 ha ha

@philippkuehn

This comment has been minimized.

Copy link
Contributor Author

philippkuehn commented Sep 26, 2018

@qyloxe Good points! I think option 2 is way harder to implement but we'll see. Have to fiddle around with it.

@qyloxe

This comment has been minimized.

Copy link

qyloxe commented Sep 26, 2018

@philippkuehn thanks! surely I plan to contribute something, because your work is remarkable and... enlightening ;)

@jaysaurus

This comment has been minimized.

Copy link

jaysaurus commented Sep 26, 2018

@jaysaurus would love to hear what you trying to do! What wasn‘t possible with tiptap?

Hiya, the issue is kind of 2 fold

  1. the slot-scope injections do not seem to allow me to update the editor with classes properly (as per my pull to you where I try and set alignment. I have tried so many different things to update classes, all to no avail). I'm not wholly sure that I can get the desired outcome without a fairly sizable refactor of the core of the build because I think the EditorView and State are being copied rather than referenced (since they are not updating the UI as expected).

  2. Just a personal thing (and I must stress that I mean no disrespect - your code is excellent), I'm not a fan of "classical" formats in JS. You may wish to investigate the compositionally-minded framework stampit as it greatly improves upon JS' difficult inheritance model should you wish to proceed in that direction.
    For my sins, I much prefer using object factories; in this fashion, I believe you can reconcile ProseMirror and Vue with less complexity and play to JS' real strengths. So it may be my own stubborn nature getting in the way as well :P

Either way, I'm midway through putting something together that only requires one simple mount and that should just work with templates etc without the need for creating lots of stuff in DOM. I'll be happy to share once I've got enough to present to you! :) I've already got a working bubble menu (with bold and italic marks) that is written with single page templates alone. Ideally, I just want to be able to expose the prosemirror's actual modules when and wherever they're needed rather than having the client have to dig for them.

@philippkuehn

This comment has been minimized.

Copy link
Contributor Author

philippkuehn commented Sep 27, 2018

@jaysaurus Hey, thanks for your thoughts!

To 1: I'm still not sure what you wanted to do with implementing text alignment and why it wasn't possible to add some classes. So I created a basic example for text-align on the demo page. I'm using inline styles there but you can achieve the same with adding classes. Basically I only needed to change two lines in the ParagraphNode extension to get this working.

To 2: I didn't know stampit but it really looks like Vue :) Do you mean that the extensions should be written in that format instead of JS classes?

@jaysaurus

This comment has been minimized.

Copy link

jaysaurus commented Sep 27, 2018

Hiya, wow thank you for such a quick response!!

to 1. I was aiming for something node independent. The idea being that - whatever types of block node - you could select any amount of text (be it h1, B, ul etc.) and align it accordingly.

to 2. it sort of depends on how you want to build it really! You like using inheritance models so one option is to go down the stampit route (because it gives you true composition in JS rather than JS' long-suffering attempt to pretend it's classical, which of course it isn't!)

Personally, I think we can use the Vue.js architecture as it is without introducing further complexity! I've thrown together the thing I'm working on as a generic example (apologies for the shonky css and the use of an id for a query selector, I'm really just prototyping at this stage but - hopefully - it gives you an idea of what I mean)

https://gitlab.com/jaysaurus/vue-with-prose-mirror

@jaysaurus

This comment has been minimized.

Copy link

jaysaurus commented Sep 27, 2018

addendum, ahhhhhh, vis a vis my first point;

I think I see now having looked at your example.

So in theory, you'd actually have to have each node type have it's own command for handling attribute changes... I see. I've been coming at it from the wrong angle (Prose Mirror likes to make things fiddly doesn't it haha!)

furthermore, I see that the prose mirror schema has to explicitly know about what attributes a block can have. Hoo boy that was complicated 😖

I've managed to get the desired behaviour in my little sandbox example (https://gitlab.com/jaysaurus/vue-with-prose-mirror) using the below as an event:

import { setBlockType } from 'prosemirror-commands'

function AlignmentFactory (className) {
  function getSelectionObject ({ $anchor, $head }) {
    return {
      from: $anchor.pos <= $head.pos ? $anchor.parent : $head.parent,
      to: $head.pos > $anchor.pos ? $head.parent : $anchor.parent,
      fromFound: false,
      toFound: false
    }
  }

  return {
    align (state, view) {
      let { from, to, fromFound, toFound } = getSelectionObject(state.selection)
      const commands = []
      state.doc.descendants((node, pos) => {
        if (!node.isInline && (node === from || (fromFound && !toFound))) {
          if (!commands.filter(({type}) => type === node.type.name).length) {
            const command = setBlockType(node.type, { class: className })
            commands.push({ type: node.type.name, command })
          }
          fromFound = true
        }
        toFound = toFound || (from === to || node === to)
      })
      // Doesn't seem to work if i run the commands inside the descendants iteration so I run it here.
      commands.forEach(({command}) => command(view.state, view.dispatch))
    }
  }
}

export { AlignmentFactory }

(not up on my public gitlab instance but I can add it should you be interested)

@philippkuehn philippkuehn referenced this issue Oct 1, 2018

Closed

Roadmap for v1.0.0 #41

5 of 5 tasks complete
@philippkuehn

This comment has been minimized.

Copy link
Contributor Author

philippkuehn commented Nov 14, 2018

fixed in v1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment