Skip to content

Instantly share code, notes, and snippets.

@phiggins42
Last active August 29, 2015 14:18
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save phiggins42/a9dd79afb4fc6feb2ec1 to your computer and use it in GitHub Desktop.
Save phiggins42/a9dd79afb4fc6feb2ec1 to your computer and use it in GitHub Desktop.

Architecture Shift

Quick little document to outline changes to the extension architecture, and internal tooling. NOTE i'm not 100% married to any of these ideas in particular, when @davi brought up "stepping back" and rethinking architecture I sat down with a bottle of bourbon and started typing. This is sorta-kinda what I would envision an API looking like (minus a few caveats of trying to retain something reselmbling a migration path forward). This is mostly just a braindump.

// this needs to work, just like this. always.
var editor = new MediumEditor(".elements", {
    // optional options / default overrides
});

Buttons defined the same:

new MediumEditor(".editor", {
    buttons:["bold","italic","underline"]
});

All the things are considered "extensions". The "extension" is minimal, it just defines the way the editor interacts with the extension. The above 3-button example would instantiate a new Button() passing in each of the definitions for bold, italic, and underline.

// extensions "can" inherit from root Extension, this is just the API def
var MyExtension = MediumEditor.Extension.extend({

    // explicit name override. defaults to the key in the `extensions` 
    // list, but allows for overriding here to allow an extension to 
    // take control over a default builtin button/etc
    name: "",

    init: function(){
        // called as part of ctor. indicates the `this.base` is _going_ 
        // to use the extension. adjust props and whatnot. `setup` will
        // be called when activating
    },

    setup: function(){
        // this.base exists, this.base.options exist
        // we are being told we are wanted as an extension to `this.base`
        // and that we should register ourselves with things and do our work
    },

    destroy: function(){
        // we are expected to cleanup any event listeners and things.
        // basically promise to undo anything we did during init phase
    }

});

Placeholders becomes an extensions. All it does is placeholders. It is 'registered' as an extension.

An idea as to how to better instantiate/define overrides/props:

var CustomExtension = MediumEditor.Button.extend({
    label:"<i class='icon-warning-sign'></i>",
    handleClick: function(e){
        // my button was clicked
    } 
});

new MediumEditor(".editor", {
    buttons:["anchor","custom-button"],
    options:{
        toolbar: false,
        placeholders: false,
        anchor: {
            targetBlank: false,
            target: false,
            anchorButton: false,
            anchorButtonClass: 'btn'
            anchorInputPlaceholder: 'Paste or type a link',
            anchorInputCheckboxLabel: 'Open in new window',
        },
        "anchor-preview":{
            anchorPreviewHideDelay: 500,
        },
        "custom-button": new CusomExtension({
            // my options
        })
    }
});

Note, the Default version of the above would look like:

MediumEditor.prototype.defaults.extensions = {
    // the default options for each of these are defined as 
    // proto props (or a special .options? meh, I like mix(this, options)
    "toolbar": true,
    "placeholder": true,
    "paste": true,
    "anchor-preview": true
}

The possible values passed to extensions object are:

  • true: enabled, no override options
  • false/undefined/null: disabled, does not instantiate.
  • object: passed as overrides to existing ctor? or as init props? replaces.
  • function: replaces/creates default ctor

So the above defaults means: a Toolbar, PasteHandler, Placeholders and AnchorPreview are all created with default props.

overall

So we end up with a namespace like:

MediumEditor = function(elements, options){
    this.elements = elements; 
    copyInto(this, options);
    this.init.apply(this, arguments);
};

MediumEditor.version = "5.0.0";

MediumEditor.defaults = {
    // existing option defaults?
    buttons:[],
    options:{
        "toolbar": true // etc
    }
};

// exposed utility and management stuff?
MediumEditor.util = Util;
MediumEditor.selection = Selection;

MediumEditor.buttons = {
    // buttonsData ... k/v pairs of "builtin" button definitions.
    // used internally to auto-instantiate Button extensions with
    // known property defaults
}

MediumEditor.Extension = function(opts){
    // same mixin pattern as core editor. opts override defaults,
    // default defined as instance/proto props
    copyInto(this, opts);
    this.init.apply(this, arguments);
}

MediumEditor.Extension.prototype = {
    // core extension api. setup/init/destroy/et al? 
}

// allow people to extend whatever
MediumEditor.Extension.extend = Util.extendify;

// the list of default loaded extensions. users can push directly here
// prior to initialization to mindlessly override. Each extension is a 
// constructor here, not instantiated. The editor will `new` it up
// when it determines it is needed for an instance. This is just a mapping
// used by statup/init cycle to determine which ctor to use for a particular
// extension. When init happens, and the list of overrides and definitions
// needed is determined we instantiate these extensions and decorate them 
// with a `.base` property.

MediumEditor.extensions = {

    // exposing the toolbar as an extension just allows someone to
    // inject a custom brand of the toolbar in stock overrides
    toolbar: Toolbar,

    // already kind-of extensions.
    placeholders: Placeholders,
    paste: PasteHandler,

    // pseudo ideas, names probably bad:
    images: ImageManager // break any image logic out? overlaps with drag?
    "drag-and-drop": DragManager // replace core usage with extension?

    // these are all "Button" subclass/extensions. replace in the same manner
    "anchor": AnchorExtension,
    "anchor-preview": AnchorPreview,

    // not in yet:
    "font-size": FontSizeExtension,
    "font-color": FontColorExtension

};

MediumEditor.prototype = {

    init: function(){
        // magic happens. determine which extensions need setup.
        // the list of things to call .init on are some funky combination
        // of default extensions -> +/- override config values -> custom 
        // extensions. 
    }

    // + existing public APIs

};

Might be helpful to peek at all the existing runtime overrides available (broken into sort-of-proposed structure)

Core

// would make more sense to implement a set("disabled", bool) ?
disableEditing: false,

allowMultiParagraphSelection: true,
buttons: ['bold', 'italic', 'underline', 'anchor', 'header1', 'header2', 'quote'],

// might consider implications of this, and provide a way away from 
// fa-bulk defaults? 
buttonLabels: false,

disableReturn: false,
disableDoubleReturn: false,

elementsContainer: false,
standardizeSelectionStart: false,
contentWindow: window,
ownerDocument: document,
firstHeader: 'h3',
secondHeader: 'h4',

... seems pretty straightforward?

anchor

anchorInputPlaceholder: 'Paste or type a link',
anchorInputCheckboxLabel: 'Open in new window',
checkLinkFormat: false,
targetBlank: false,
anchorTarget: false,
anchorButton: false,
anchorButtonClass: 'btn',

all of those seem like perfect candidates for AnchorExtension.prototype

anchor-preview

// anchor-preview
anchorPreviewHideDelay: 500,
disableAnchorPreview: false,

The disabled option would be deprecated. instead, during init they'd pass false to the anchor-preview extension:

new Editor(".a", { extensions: { "anchor-preview": false } });

Overwriting the anchorPreviewHideDelay would be:

new Editor(".a", { extensions: { 
    "anchor-preview": {
        "hideDelay": 1500
    }
} });

would detect disableAnchorPreview===true and deprecate-warn, and remove default "anchor-preview" extension from being activated

paste

cleanPastedHTML: false,
forcePlainText: true,

these might still be core options really. but if paste is built out as an extension and exposed outright, any paste related-options could be defined on the "paste" override:

new Editor(".thing", {
    extensions:{
        "paste": {
            // weirdness here. if you "paste":false, it would make the extension
            // not load at all, so forcePlainText et all would not even be 
            // considered? this extension seems to need to be on all the time
            // anyway? 
            clean: true,
            forcePlainText: false,
            omitTags:["b","h3","pre"]
        }
    }
});

toolbar

disableToolbar: false, // -> deprecated
toolbarAlign: 'center', // -> `align`
delay: 0,
diffLeft: 0,
diffTop: -10,
activeButtonClass: 'medium-editor-button-active',
firstButtonClass: 'medium-editor-button-first',
lastButtonClass: 'medium-editor-button-last'

placeholders

disablePlaceholders: false, // does this disable the palceholder in anchor?
placeholder: 'Type your text',

disablePlaceholders deprecated, pass false to "placeholder" extension. can do deprecation version easily. placeholder moved to property of the placeholders extension

image? drag-and-drop?

imageDragging: true,

So the existing defaults become:

{
    extensions: {
        "image":{
            "dragging": true
        },
        "placeholders":{
            placeholder: "Type your text"
        },
        "toolbar":{
            align: 'center', // -> `align`
            delay: 0,
            diffLeft: 0,
            diffTop: -10,
            activeButtonClass: 'medium-editor-button-active',
            firstButtonClass: 'medium-editor-button-first',
            lastButtonClass: 'medium-editor-button-last'
        },
        "paste":{
            clean: false,
            plainText: true
        },
        "anchor":{
            inputPlaceholder: 'Paste or type a link',
            inputCheckboxLabel: 'Open in new window',
            checkLinkFormat: false,
            targetBlank: false,
            anchorTarget: false,
            anchorButton: false, 
            anchorbuttonClass: 'btn'
        },
        "anchor-preview":{
            iideDelay: 500
        }
    },

    allowMultiParagraphSelection: true,
    buttons: ['bold', 'italic', 'underline', 'anchor', 'header1', 'header2', 'quote'],

    // might consider implications of this, and provide a way away from 
    // fa-bulk defaults? 
    buttonLabels: false,

    disableReturn: false,
    disableDoubleReturn: false,

    elementsContainer: false,
    standardizeSelectionStart: false,
    contentWindow: window,
    ownerDocument: document,
    firstHeader: 'h3',
    secondHeader: 'h4'
}
@nmielnik
Copy link

I'm definitely on board with these suggestions, I think this helps out with our large pile of options, and also helps us standardize the lifecycle of our built-in extensions. This is a great end-goal for us, I'd like to see what Davi and Noah think.

Ultimately, I think this would be 4 separate sets of changes:

  1. Introduce the extend method to our existing extensions as a simpler way to make your own extensions that build on top of the default behavior.
  2. Introduce the nested options that you called out here, and have those passed into the built-in extension via the constructor. You didn't cover how people would pass their own custom buttons/extensions in, which would be an additional case where the key is a string and the value is an instantiated object. If at all possible, we'd want to be backwards compatible with how options were passed in before, which might take some thought but would be worth keeping around for a while.
  3. Standardize on the destroy and setup methods instead of activate and deactivate for the extensions. I'm not sure if we'll need to "deprecate" vs "delete" them or not, as the introduction of these methods on extension themselves was only added recently.
  4. For the Toolbar, Figure out just how we want to make the toolbar changeable. It would be good to have an idea of just how we might want this to be changeable. Based on a the type of users most of our user base consists of, it might be better to try to expose options that can be customized here first in addition to make the Toolbar extendable.

@phiggins42
Copy link
Author

Introduce the extend method to our existing extensions as a simpler way to make your own extensions that build on top of the default behavior.

is how this all started, really. @davi suggested stepping back and rethinking how options and .base decoration happens, so I spewed out the above. most of the above could be introduced without breaking back compat (with deprecation warnings/etc), but some is an outright shift, and no sense in preserving a broken pattern if it is in fact deemed broken and wanting replaced.

Introduce the nested options that you called out here, and have those passed into the built-in extension via the constructor. You didn't cover how people would pass their own custom buttons/extensions in, which would be an additional case where the key is a string and the value is an instantiated object. If at all possible, we'd want to be backwards compatible with how options were passed in before, which might take some thought but would be worth keeping around for a while.

ultimately the nested options are just moving properties from one place to a newly named place. it is a little fuzzy here. the whole nested-options thing came out of @davi mentioning specifically "the way options were passed in" ... I immediately noticed the inconsistency in how various internal-things pass instance and options ... then fell back to the pattern of: each extensions 'options' are just prototype properties, and there are no explicit options to speak of. the "options" being passed to the constructor is just copyInto(this, options). so ...

var Thing = MediumEditor.Extension.extend({
       configOptionOne: true,
       configOptionTwo: false,
       init: function(){
          // this extension was needed/requested. I have `this.base.options` and `this.base` available to me
          // and all of my own options are defined as `this.something`
       }
});

So the above extension has two configuration options, one defaults true the other false.

new Editor("...", {
     extensions:{
          // current way, mostly:
          "magic": new Thing({ configOptionTwo: true }),

          // or passing a new Ctor
          "differentmagic": Thing.extend({
              configOptionTwo: true
          }),

          // or with a prop override (these are passed to ctor like `magic` example)
          "examplemagic": {
                // but the extension has to be on default extensions prior for this to even be a thing
                // MediumEditor.extensions.examplemagic = Thing;
                "configOptionTwo": true
          },

          // or the case where they simply want an existing .extensions member to be included
          // also requires MediumEditor.extensions.thing = Thing;
          "thing": true 
     }
});

revolves around there being no options per-se being passed via ctor, just potential overrides (making them one in the same). The weirdest bit would be discovery between the magic and differentmagic patterns, where on is a ctor and the other already-instantiated?

For the Toolbar, Figure out just how we want to make the toolbar changeable. It would be good to have an idea of just how we might want this to be changeable. Based on a the type of users most of our user base consists of, it might be better to try to expose options that can be customized here first in addition to make the Toolbar extendable.

I think moving the toolbar-specific options to Toolbar.prototype overrides, and exposing as an extension is "enough" for first pass. Discovering how people want to use it will only come after they are able to use it. I have my own desires for a replaceable toolbar, and would be happy to share :)

All of the option-moving can be done as s deprecation pass ... though the use of an explicit options passed to any of the extensions would be a breaking change, but the ability to do exactly the same by mixing over your extension .proto (or inheriting from Extension directly instead of making your own ctor) is a pretty easy migration path for extension authors.

@phiggins42
Copy link
Author

Standardize on the destroy and setup methods instead of activate and deactivate for the extensions. I'm not sure if we'll need to "deprecate" vs "delete" them or not, as the introduction of these methods on extension themselves was only added recently.

agree, the extensions should provide lifecycles methods that match the overall editor. would ultimately require a peek at the various ways extensions are expected to behave. in the end it's just names, though naming things is hard :) would love to define that api as part of the Extension api, startup/init/destroy/disable(or set(disabled, ?))/kinds of things ... defining and documenting them as core extenstion lifecycle would open the floodgates to external functionality and stabalize a bunch I think

@daviferreira
Copy link

I really like it. I think we could go even further as make the toolbar an extension.

This is what I have always sketched when thinking about refactoring MediumEditor: we would have the core editor (that would represent the contenteditable now, maybe the content model in the future), extensions and buttons/actions. Extensions would be stuff like placeholder, paste, drag and drop images, anchor preview, toolbar... While buttons/actions would be stuff like anchor form, tables, fonts etc. and would have a standard UI (button/toolbar back).

They of course could share a base functionality/prototype (not sure what to call it).

Let's keep the discussion going and come up with a feasible plan 🍺

@daviferreira
Copy link

Also, MediumEditor always has been pure javascript, with no external libs, but maybe its time to rethink it aswell. I already mentioned to Nate and Noah that maybe we should consider using Rangy for the selection part, for example.

Another thing is to have a more structured internal event system (or pub/sub thing).

With all that in mind, there is only one thing that I really would like to keep: it should be easy to contribute to the project.

@nchase
Copy link

nchase commented Apr 3, 2015

👍

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