MAMABA / editstrap

Make editable field using jquery and bootstrap
http://www.editstrap.com
12 stars 1 forks source link

More callbacks? #42

Open mhulse opened 7 years ago

mhulse commented 7 years ago

Would be nice to have more callbacks/hooks that get fired off throughout the codebase.

Like, beforeEdit would be pretty useful.

mhulse commented 7 years ago

Ah, I see an undocumented this.options.beforeSendUpdate(). That's going to be a good one to use. You should document this.

Looks like that might be the only undocumented callback?

Would love to have more callbacks! You can't go wrong with having them.

Also, rather than do:

if (this.options.beforeSendUpdate != undefined) { }

You could just have this in your defaults object:

beforeSendUpdate: $.noop,

Which makes it more clear to other devs that there's a callback for use at that point. The $.noop allows you to skip the conditional check as it is just an empty function.

With all that said, +1 for more callbacks throughout your codebase.

mhulse commented 7 years ago

This is nitpicky, but … Conventionally, I prefer to use the word "on" for things like onBeforeSendUpdate.

mhulse commented 7 years ago

Currently you have this:

this.options.beforeSendUpdate(this.$element, dataToSend);

You might consider using call() instead:

this.options.beforeSendUpdate.call(this, dataToSend);

Then in the user’s init code:

$('span.editstrap').editstrap({
    type: 'text',
    url: '/api/endpoint',
    beforeSendUpdate: function(dataToSend) {

        console.log(this.$element, dataToSend);

    }
});
mhulse commented 7 years ago

Sorry to keep spamming with ideas, but another useful callback could be:

onBeforeInit

That way one could set the "type" via a data attribute like this.$element.data('type') and perhaps we would have more of a dynamic way of doing things?

mhulse commented 7 years ago

You could also just allow users to define type via a data attribute JSON object.

Here's an example of merging data attributes with plugins defaults/options/etc.:

settings = $.extend(false, {}, defaults, options, $this.data(NS + 'Options')); // Recursively merge defaults, options and data attribute options.

And in the html:

<section class="ion" data-ion-options='{ "allowMultiple" : false, "alwaysOpen" : true }'>
    ...
</section>
mhulse commented 7 years ago

Useful:

  1. Defaults:
onBeforeInit: $.noop,
onAfterInit: $.noop,
onBeforeSendUpdate: $.noop,
  1. Init method at top:
init : function(e) {

    this.options.onBeforeInit.call(this);
  1. Init method at bottom:
    this.options.onAfterInit.call(this);

},

Bonus! Changing beforeSendUpdate to match the others (and gives developers more access to all of the options):

this.options.onBeforeSendUpdate.call(this, dataToSend);

Now, my initialization code:

$(function() {

    // https://github.com/MAMABA/editstrap/issues/42
    $('.editstrap').editstrap({
        type: 'text',
        url: '/api/placeholder', // This is needed, otherwise editstrap won't think it's an Ajax field.
        onAfterInit: function() {
            this.options.type = (this.$element.data('type') || this.options.type); // Get the API url via the form's action attribute, or use the default.
        },
        onBeforeSendUpdate: function(dataToSend) {

            this.options.url = this.$element.closest('form').attr('action');

            console.log(this.options.url);

            dataToSend[this.$element.data('name')] = dataToSend.value;

        }
    });

});

Last, my template:

<form action="/api/endpoint">
<span class="editstrap" data-name="last_name" data-type="textArea"><%=last_name%></span>
</form>

Of course, if you headed the data attribute JSON object route, the above approaches would change to match. You could have a data attr like:

<span data-editstrap-options='{
    "option": "value",
    "option": "value",
    "option": [
        ...
    ]
}'>

Or, have data attribute for each option, like:

<span data-editstrap-type="..." data-editstrap-url="...">
mhulse commented 7 years ago

Some more:

onBeforeOpen: $.noop,
onAfterOpen: $.noop,
onBeforeClose: $.noop,
onAfterClose: $.noop,

Then:

this.$element.parent().click(function(e) {

    _this.options.onBeforeOpen.call(_this, this);

// ....

    _this.options.onAfterOpen.call(_this, this);

});

And:

if (element.attr('data-editable-active') == 'true') {

    this.options.onBeforeClose.call(this, element, text);

// .....

    this.options.onAfterClose.call(this, element, text);

}

Finally:

$(function() {

    // https://github.com/MAMABA/editstrap/issues/42
    $('.editstrap').editstrap({
       // ......
        onBeforeOpen: function(parent) {
            console.log(this, 'onBeforeOpen', parent);
        },
        onAfterOpen: function(parent) {
            console.log(this, 'onAfterOpen', parent);
        },
        onBeforeClose: function(element, text) {
            console.log(this, 'onBeforeClose', element, text);
        },
        onAfterClose: function(element, text) {
            console.log(this, 'onAfterClose', element, text);
        }
    });

});
mhulse commented 7 years ago

A couple more new callbacks:

onBeforeGetHtml: $.noop,
onAfterGetHtml: $.noop,

Then:

getHtml : function getHtml(value, span) {
    this.options.onBeforeGetHtml.call(this, value, span);

// ....

    this.options.onAfterGetHtml.call(this, html);
    return html;
}

Finally:

$(function() {

    // https://github.com/MAMABA/editstrap/issues/42
    $('.editstrap').editstrap({
        // ...
        onBeforeGetHtml: function(value, span) {
            console.log(this, 'onBeforeGetHtml', value, span);
        },
        onAfterGetHtml: function(html) {
            console.log(this, 'onAfterGetHtml', html);
            // https://github.com/MAMABA/editstrap/issues/40
            html.removeClass('input-group-sm');
        }
    });

});