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

consider adding a "commands" concept #2066

Closed
ianstormtaylor opened this issue Aug 9, 2018 · 11 comments
Closed

consider adding a "commands" concept #2066

ianstormtaylor opened this issue Aug 9, 2018 · 11 comments

Comments

@ianstormtaylor
Copy link
Owner

ianstormtaylor commented Aug 9, 2018

Do you want to request a feature or report a bug?

Feature idea.

What's the current behavior?

Currently we have three concepts that all contribute to "editing" in Slate editors:

  • Event handlers, whose job is to intercept user input and translate it into changes to the editor's content that Slate understands.

  • Changes, whose job is to provide high-level ways of manipulating the editor's content, so that event handlers (and other programmatic use cases) can easily apply changes.

  • Operations, whose job is to convey the lowest-level description of the individual transformations that were applied to the editor's content, for use in collaborative editing.

However, all of this currently rests on the ability of event handlers to be able to easily read user intent. Sometimes that's easy, for example detecting when enter is pressed. But sometimes it can be very convoluted, which leads to a few different problems...

  1. In "soft keyboards", certain key presses don't actually fire with complete information (because they can't), so you can no longer detect keys like backspace being pressed purely from the keydown event. Getting around this would require all plugins to handle the complexity of backspace detection themselves.

  2. When plugin's want to modify behaviors that maybe interact with other plugins, there's no way for one plugin to guarantee that it has hooked into all instances of a particular user intent. For example, a plugin may listen for cmd+b to catch "bold" intent, without knowing that another plugin introduced a shortcut that makes cmd-* also trigger bold formatting. This isn't solvable without an extra "intentful" layer.

  3. Browsers differ as to which behaviors they support natively in contenteditable elements, and some browsers provide extra UI for certain actions. For example, iOS provides a contextmenu that includes B/I/U for formatting. Since Slate is schema-agnostic, there's no way to translate the "schema-specific" actions from these context menus into behaviors in a Slate editor.

There might be a way to solve this...

What's the expected behavior?

We'd introduce a new layer of logic for editing that is focused on user intent. One one side of it would be the middleman between event handlers, browser-specific contextmenu UIs, and other client-specific behaviors. And on the other side, would be the Slate-specific changes and operations that change the editor's content.

Based on @majelbstoat's previous discussion, it think it could be good to call it "commands". This also maps nicely to browser's concept of execCommand.

It would be similar in concept to the beforeinput/input events from the Input Events spec, except that it would be browser and device agnostic, and it would be extendable by developers and plugins.

For example, consider mapping many different inputs to the same 'bold' command...

cmd+b -> bold
** -> bold
click bold button -> bold

And then defining what 'bold' does in a single place:

onCommand(command, change, editor) {
  switch (command.type) {
    case 'bold': {
      change.toggleMarkAtRange(command.range, 'bold')
      return
    }
    ...
  }
}

This seems good. But it brings up some questions:

  • Should commands all take a range object that defaults to the current selection? This seems like a good way to do it, especially since that's how InputEvent works, where each action corresponds to a range in the document (collapsed or not). Or, more simply, are commands always performed in the context of the user on the current selection?

  • How should actions take in extra information about the action? For example, for inserting text we need a way to specify the text to insert. For dynamic mark/blocks we might need a way to pass in mark.data or block.data properties. The InputEvent spec handles this by using a combination of event.data and event.dataTransfer properties, so we might be able to get away with a single options/data dictionary.

  • Might this eliminate the need for the "current selection transforms"? We have things like change.insertText(...), but if there's a command for it instead, we could just do editor.command('insert_text', ...) instead and simplify the number of concepts.

  • How would commands define their "merging" behaviors with previous commands? For example if two insert_text commands occur adjacent to each other, they should be merged into a single entry in the history's undo stack (like other editors).

  • Do we need an equivalent of the document.queryCommandState API for determining which way certain commands will toggle? For use in determining whether toolbar buttons should be active or not for example.


@majelbstoat you mentioned in #1730 that you use this pattern in Medium. Is there any way you could describe your architecture/findings? Any information would be super helpful!


Existing Issues

@Nantris
Copy link
Contributor

Nantris commented Aug 9, 2018

I haven't read this whole thing, but I like the idea.

One thing I'd like to implement on Windows is the option to use Ctrl+Shift+Z for redo, but the proper default action is to use Ctrl+Y for Windows. This should not change in Slate, but it seems like this idea would make supporting Ctrl+Shift+Z easier, among lots of other potential uses.

@YurkaninRyan
Copy link
Collaborator

I attempted to do exactly this when building my editor, but I had to make certain changes depending on context as well.

For example we have the concept of a "slash command" in our editor. So when you fired the "become paragraph" command, you would also have to delete the /paragraph text that was in the block you were typing in

@ianstormtaylor
Copy link
Owner Author

Just food for thought...

I wonder if there is also a way that commands could make it so that lots of the monotonous and common behaviors for editors can be even more streamlined... for example, when defining a new type of Mark, you'll end up having to write a bunch of your own change functions:

function addCodeMark(change) {
  change.addMark('code')
}

function removeCodeMark(change) {
  change.removeMark('code')
}

function toggleCodeMark(change) {
  change.toggleMark('code')
}

Just so that it's easier to share these pieces of logic around your codebase without having to define them in multiple places.


But now separately, consider that if we do have "commands", one of the benefits will be that all of the native browser behaviors from native execCommand-like UI menus can be nicely intercepted and handled gracefully.

For example, we'd want to make it such that if a user presses the B button in the iOS context menu, and their editor supports bold formatting, that it toggles bold. Just like it would in a normal contenteditable editor.

Or for example, we'd want to make it such that shake-to-undo works, by funneling those events into the "undo" command that Slate defines.

But at the same time, we want to keep Slate's "schema agnostic" guarantee. So by default, there's no "bold" logic in the editor, so the "bold" command does nothing.

I think this could be solved via canonicalizing the command names. For example, if core happens to trigger a 'bold' command for iOS context menus, everyone will end up using that command name because to use something different would just break compatibility.


But, taking these two ideas together... what if the commands were actually smartly created just by adding marks/blocks/etc to the schema. For example, defining a schema of...

{
  marks: {
    bold: {},
    italic: {},
  }
}

Would automatically start handling the following commands:

switch (command.type) {
  case 'add_bold': return change.addMark('bold')
  case 'remove_bold': return change.removeMark('bold')
  case 'toggle_bold': return change.toggleMark('bold')
  case 'add_italic': return change.addMark('italic')
  case 'remove_italic': return change.removeMark('italic')
  case 'toggle_italic': return change.toggleMark('italic')
}

If this were the case, there wouldn't be a need to go to the trouble of defining and sharing these addBoldMark, toggleItalicMark change helpers, because the command API would already made encapsulating commands possible.

So you could instead just do:

editor.command('toggle_bold')

I know from my existing use case that this would remove a lot of code.

But still a question if it's too much magic.

@YurkaninRyan
Copy link
Collaborator

Really at this point it just feels like we are firing actions, and the command handler is just a reducer. I think introducing magic there would actually limit users. We might reduce a lot of code, but use cases outside of our own might be hurt.

It sounds like we can extract the most value from having standardized into action names. And a way for plugins to reference other plugins action names.

For example if I want to hijack slate-edit-list's command to break and make a nest list item, I could just make a command that that grabs it's command action, and handles it. Then put my plugin in the correct spot in my plugins order.

@Nantris
Copy link
Contributor

Nantris commented Aug 12, 2018

We might reduce a lot of code, but use cases outside of our own might be hurt.

@YurkaninRyan you might be right, but I'm not entirely sold one way or the other.

Can you think of a specific example where this would interfere with the overarching goal to be an agnostic extensible contentEditable?

@YurkaninRyan
Copy link
Collaborator

@slapbox Sticking with the redux analogy, it would be as if redux came out of the box with pre-defined actions that are already tied to pre-defined reducers.

The true value is just in having a way to expose and communicate user intent, I think that's the sweet spot in this feature proposal. I think adding a portion that reduces boilerplate could be useful, but there needs to be a way for someone to overwrite or eject that pre-defined behavior.

I think slate does best when it's not opinionated about things, and if it is, it provides a good escape hatch

@ianstormtaylor
Copy link
Owner Author

@YurkaninRyan I think I agree with you.

The only place I still get tripped us is how Slate's core can transform things like execCommand('bold') into actions, without being opinionated, or having some sort of "best practice" in terms of how things are named.

But thinking about it more... given that the current plugins in slate-react may be renamed to slate-dom in the future... maybe there's also a slate-dom-extended (or something) that allows for those more opinionated native commands to be caught and processed by Slate, and that plugin takes in options for how to do that. That could work.

Just a question to me still whether it's worth the indirection to avoid the opinion.

@YurkaninRyan
Copy link
Collaborator

@ianstormtaylor is the idea that someone from outside of slate could call execCommand and we would intercept and give users a chance to override it? Or are you just referencing how that api actually works

@ianstormtaylor
Copy link
Owner Author

ianstormtaylor commented Aug 15, 2018

@YurkaninRyan essentially the first one. The [Input Events Level 2]https://www.w3.org/TR/input-events-2/( specification defines a bunch of beforeinput events that actually corollate to formatting/block-specific behaviors like formatBold, formatItalic, insertOrderedList.

(Edit: and to be clear, browser-level UI like iOS's toolbar, or the right-click contextmenu may create events with these input types.)

I'm wondering if there's a way we can square those, without it requiring a plugin. Or create some sort of standard around what the names should be. But it very well might be the case that it just introduces too much magic. 😄

@justinweiss
Copy link
Collaborator

justinweiss commented Sep 4, 2018

I love this idea. I think it would help solve a problem I've run into a lot with our editor around tables.

For example, if you have a 2x2 grid selected out of a 4x4 table, the "bold" command should only bold those cells, and the "bold" toolbar item should only be highlighted if all text in all of those cells is bold. This means that many of the toggle / set / is methods have to be wrapped in logic to detect whether there's an active cell selection, and change their behavior if that's the case.

If, instead, the table plugin could listen to commands and do the correct thing, it would simplify how those commands are handled, and also allow all of that logic to stay inside the table plugin, instead of in the global plugin configuration, like it is today.

I'd also be 👍 for native controls sending commands, and it could be up to the editor to decide how to handle them.

@ianstormtaylor
Copy link
Owner Author

@justinweiss I was thinking about something for the toolbar button active states... right now these are determined by one-off functions everywhere. But if we introduce the idea of a "command stack" it seems like we might also need a "query stack" (or similarly named) for determining active states. Something similar to queryCommandState potentially.

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

No branches or pull requests

4 participants