Flyer53 / jsPanel4

A JavaScript library to create highly configurable floating panels, modals, tooltips, hints/notifiers/alerts or contextmenus for use in backend solutions and other web applications.
https://jspanel.de/
Other
317 stars 57 forks source link

jsPanel.modal.create(opts, cb, cb2) ignores cb and cb2/ improved callback suport #196

Closed syonfox closed 2 years ago

syonfox commented 2 years ago

Describe the bug It seems there is an inconsistency with how additional params are handled.

To Reproduce

jsPanel.create

let config = {
                headerTitle: 'Foo Test',
                content: 'fucking chaos',

                callback: [
                    (p) => {
                        console.log('Step: callback');
                        return 'whatever'
                    },
                    (p) => {
                        console.log('Step 2: callback');
                        return false;
                    },
                    (p) => {
                        console.log('Step 3: callback');
                        return true;//ignored all are executes
                    }],
                onbeforeclose: [
                    (p) => {
                        console.log('Step: before close');
                        return false
                    }, (p) => {
                        console.log('Step 2: before close');
                        return true;
                    },(p) => {
                        console.log('Step 3: before close');
                        return true; // not checked becuse we know we are closing by step2
                    }],
                onclosed: [
                    (p) => {
                        console.log('Step: close');
                        return true
                    }, (p) => {
                        console.log('Step 2: close');
                        return false;
                    },
                    (p) => {
                        console.log('Step 3: close (dont execute cust step 2 failes)');
                        return true;
                    }],
            }
            jsPanel.create(config, ()=>{console.log("one more cb ")},()=>{console.log("one more cb 2")})

VM223113      Step: callback
VM223113:11 Step 2: callback
VM223113:15 Step 3: callback
VM223113:42  one more cb 

VM223113:20 Step: before close
VM223113:23 Step 2: before close
VM223113:31 Step: close
VM223113:34 Step 2: close

jsPanel.modal.create

jsPanel.modal.create(config, ()=>{console.log("one more cb ")},()=>{console.log("one more cb 2")})
Step: callback
VM223933:11 Step 2: callback
VM223933:15 Step 3: callback

VM223933:20 Step: before close
VM223933:23 Step 2: before close
VM223933:31 Step: close
VM223933:34 Step 2: close

Expected behavior I would expect my callback (one more callback) to be called in jsPanel.modal.create

Recommendation

support endless callback args in a nice way. bonus: support

Notes in this demo function i replaced 'function' with 'number' to test

    function optionsAndCallbacks(options, ...callbacks) {
                options = options || {}
                console.log("opt: ", options);
                console.log("cbs: ", callbacks)
                let goodCallbacks = [];
                function validateCb(cb) {
                    if(typeof cb == "number") {
                        goodCallbacks.push(cb);
                        return;
                    }
                    if(Array.isArray(cb)) {
                        cb.forEach(validateCb);
                        return;
                    }
                    console.warn("Invalid Callback Provided (skipping): ", cb);
                }
                callbacks.forEach(validateCb);

                validateCb(options.callback)

                console.log("good cb",goodCallbacks)
                //now all callbacks are in the array goodCallbacks

            }
            optionsAndCallbacks();
            optionsAndCallbacks({callback:1}, 'wrong');
            optionsAndCallbacks({callback: 6}, 1,2,3,[4,5] )
            optionsAndCallbacks({callback: [6,7]}, 1,2,3,[4,5] )

opt:  
{}
VM223066:4 cbs:  
[]
VM223066:15 Invalid Callback Provided (skipping):  undefined
VM223066:21 good cb 
[]

VM223066:3 opt:  
{callback: 1}
VM223066:4 cbs:  
['wrong']
VM223066:15 Invalid Callback Provided (skipping):  wrong
VM223066:21 good cb 
[1]

VM223066:3 opt:  
{callback: 6}
VM223066:4 cbs:  
(4) [1, 2, 3, Array(2)]
VM223066:21 good cb 
(6) [1, 2, 3, 4, 5, 6]

VM223066:3 opt:  
{callback: Array(2)}
VM223066:4 cbs:  
(4) [1, 2, 3, Array(2)]
VM223066:21 good cb 
(7) [1, 2, 3, 4, 5, 6, 7]

Other Topic, jsPanel.prompt

IN other notes spent that last few days figure this out, writing a jsPanel.prompt Extension

Looking forward to sharing when code is ready i think these features will be nice but is more work then i think, will have a minimal prototype soon ish though hopefully will make a new issue.

     /*
            What features to add

            return promise
            submit -> resolve
            close -> reject

            submit: 'string' this is the button for submition also accepts button config
                    //defults to adding a submit button to footer

            cancel: 'string' aditional button to close, also accepts an array of buttons

            Add button support
                headerButtons:
                footerButtons

            Add form support
                template

            Add notification support
                notifySubmit
                notifyCancel

                */
            let defaults = {
                headerTitle: 'We Need Your Input Please, Thank You!',
                submit: 'Submit',//quick footerButton1
                cancel: 'cancel',// quick footerButton2
                notifySubmit: false,
                notifyCancel: true,

                template: false,
                content: '',
                footerButtons: [],//must be array, of items
                headerButtons: [],//must be array of items
                //item (name, el, event, fn, options)
                // el = html or dom;

                // onsubmit: [resolve],
                // oncancel: [reject],
                // onbutton: undefined, //button event,// all button
                // onchange: undefined, //form input change, //all input

                //docs
                onclosed: false, // fns added to end of chain after reject,
                onbeforeclose: false,  // if ANY 'validation functions' returns true then we continue to close. modal (careful)

                confirmClose: true, // ask user to confirm berfore closing
                confirmSubmit: true,//optionsal function to hyjack before resolve and do a validation before its too late
                    //defaults to a summary of data

                headerToolbar: [],// items appended after buttons default float left
                footerToolbar: [],// items appended after buttons defult float right
            }

            /** *****************************************
             *   * headerTitle                   _ [] X  *
             *   ---------------------------------------
             *   * [btn1][btn2]                          *
             *   ---------------------------------------
             *   * content <whaterver html>              *
             *   * <form>                                *
             *   * appendHTML                            *
             *   * template                              *
             *   * prependHTML                           *
             *   * </form>                               *
             *   ----------------------------------------
             *   *            [btn3] [cancel] [subbmit]  *
             *   *****************************************/
syonfox commented 2 years ago

For my lib, i have writen the fallowing utility. needs a little testing but i think it could work well for the jsPanel standard. what do you think?

/**
 * Utillity too get options object and callbacks from an array of arguments
 * @example
 *  function foo() {
 *      let options = parseOptionsCallbacks(arguments);
 *
 *      let rets = []
 *      options.callback.forEach(cb=>{
 *          let ret = cb();
 *          rets.push(ret);
 *      })
 *      return rets
 *  }
 * @param args, array of arguments or spread arguments [options cb1, cb2,...,cbn]
 * @return {{}}
 */
function parseOptionsCallbacks(...args) {
    if(Array.isArray(args[0])) args = args[0];

    let options = {}
    let callbacks = [];
    if (typeof args[0] == 'object') {
        options = args.shift();
    }

    function validateCb(cb) {
        if (typeof cb == "function") {
            callbacks.push(cb);
            return;
        }
        if (Array.isArray(cb)) {
            cb.forEach(validateCb);
            return;
        }
        console.warn("Invalid Callback Provided (skipping): ", cb);
    }

    //asume the rest of the args are callbacks
    callbacks.forEach(validateCb);
    validateCb(options.callback); // check for callbacks in the options

    options.callback = callbacks
    return options
    console.log("good cb", goodCallbacks)
    //now all callbacks are in the array goodCallbacks

}
Flyer53 commented 2 years ago

@syonfox Wow ... lot of stuff to look at 😏

The callbacks: If I understand you correctly you talk about the callback as in

jsPanel.create( { /* options */ } , callback );

I guess you found this callback in the jsPanel script since this callback is not part of the documentation. It's still in the script, but I never really came across a use case where this callback is actually needed. The official option.callback is enough I think. So my first question would be: is this callback capable of doing anything the regular option.callback or any other of the documented callback options can't?

And because I never had a use for this jsPanel.create( { /* options */ } , callback ); callback I did not include processing such a callback in jsPanel.modal.create() (or any other extension). Again I would ask whether it could do anything not possible with the documented callback options?


jsPanel.prompt Did you take a look already at the official dialog extension which is Promise based and provides a method

jsPanel.dialog.prompt(html, preset = "", options = {})

as well as methods jsPanel.dialog.alert(html, buttons = ["OK"], options = {}) jsPanel.dialog.confirm(html, no = false, moreButtons = [], options = {}) jsPanel.dialog.modal(html, options = {})

To see the docs and some examples please got to https://jspanel.de/#extensions/dialog

syonfox commented 2 years ago

yeah sorry for the wall of code... I do agree that is a very minor thing haha. It just threw me off because I tried to use it. but yeah. this is more advanced then necessary and i dont thing theirs much advantage of having infinite callbacks.

The main advantage is callback hell is one fewer levels deep.

I think just adding the fallowing might be a nice touch so its the same as jsPanel.create

if(arguments.length > 1 && typeof  arguments[1] == 'function') options.callback.push(arguments[1])

Thanks yeah jsPanel.dialog is a better starting point it has done half of the work. The main addition prompt would have would be a form builder component of some sorts. Im thinking maybe calling it jsPanel.form might be a better option, could depend on al of jspanel and bundle it into a single jsPanel.heavy.js or something with all extensions included :).

Off for thanks giving but will do a solid push on some elegant jsPanel signatures soon.

Just need to sort out events. I'm thinking to use Observable for a framework like https://aurelia.io/

This is an sample that was working before I rewrote everything and broke it.

jsPanel.prompt.form({
    title: "Gather Ski Prefrence",
    template: {
        ski: 'checkbox',
        board: 'checkbox',
        msg: {
            label: "Whats Up?",
            inputType: 'text'
        }
        birth_day: {
            type: input,
            inputType: 'date'
        }
    }

    appendHTML: '<p>The wost day of skiing is still better then the <b>best</b> day of almost anything else!</p>'
}).then(data=>{
    if(data.ski) console.log("My boi")
    console.log("data like template: ", data);
})catch(e=>{
    console.error("User closed modal: ", e);
})

about to hit the road off for the weekend Happy holidays

syonfox commented 2 years ago

as far as an argument for including at least one extra callback I think this snip it shows clearly how it can be a nice clean flow. imagine typing out the line of code. This is just a quality of life improvement for "extension makers" who wish to wrap jsPanel

    jsPanel.form = {
        /**
         * create a form jsPanel;
         * @param options
         */
        create(options, ...callback) {
            options = parseOptionsAndCallbacksFromArguments([options, ...callback])

            jsPanel.create(options, (panel)=>{
                // my extra callback nice and clean
            })

            // vs 
            //if array push if function make array and push.  We dont want to enforce the useres to nesasaraly append.
            let mynewcallback = ()=>{

            }
            // best case its a complex line thats not nessasary
            options.callback = typeof options.callback=="function"? [options.callback, mynewcallback] : Array.isArray(options.callback) ? options.callback.push(mynewcallback) : [mynewcallback]
            jsPanel.create(options)
            // I can avoid this whole check if I append my callback in a non standard place. like as the next unused param.

        }
    }

Hope this makes scene

Flyer53 commented 2 years ago

@syonfox That's what I like most:

<p>The worst day of skiing is still better then the <b>best</b> day of almost anything else!</p> 😄

I'd just say any outdoor time you spend in the mountains 😏


Well, concerning the jsPanel.create(options, callback) callback I see two options:

  1. either I remove it completely
  2. or I process them like in onbeforeclose for example, meaning an array of functions passed to Array.prototype.some() in order to make it consistent with the other options processing an array of callbacks. In this case I could think about supporting this kind of callback to the extensions as well.

But since I don't see a real benefit in this callback compared to the regular option.callback I tend to 1


For any questions or feature requests concerning jsPanel.dailog I'd ask you to - in case you decide to use it - contact the creator of the extension: https://github.com/daumling/jsPanel-dialog

syonfox commented 2 years ago

True, I still maintain that everyone needs a health sense of awe on their life.

Personally I think option 2 is better, sorry for the rushed response Friday but to clarify the point. By allowing 2 different spots where callbacks can be provided it essentially let's the developer offload the logic of merging an array of callbacks and thusly avoid performing validation on their options.callbacks, arg[1] before submitting jspanel. Since one can assume jspanel will call both sets of callbacks. You can simply put your logic in the added callback and pass on options. I think it's a usefully feature but mostly for developing extensions/ adding custom options to jspanel etc. Maybe it should remain un document or have anote be made. Recommending people use options callback only in extensions as the argument may not be implemented by extensions.

I suppose a simple utility function to merge callbacks may mitigate this, really this is the internal solution it may be nice to expose this function for devs to use.

mergeFunctionArray(a1,a2)
Validate a1
Validate a2
Return [...a1, ...a2]

//Alternatively
AddCallbackToOptions(options, CB){
Validate Options.callback
If CB function
Options.callback.unshift(CB)
}

As far as using array.some([cb1,cb2,cb3]) does this produce the effect of if cb2 returns true it does not continue down the chain.(like e.stopPropogation()??) It doesn't seem to work this way currently so that may break stuff. (See first jsPanel.create https://github.com/Flyer53/jsPanel4/issues/196#issue-1400411872 example where it called all of the callbacks even though the second one returns true.

... And if we are adding support for an array of CBS in arg[1] we might as well support multiple args since it's just an array ;)

I'd say ether leave it as a hidden feature in base jsPanel create only for extension developers. Warning users not to use it.

Or improve and standardized how it works so extension developers can offer the same interface easily.

Flyer53 commented 2 years ago

@syonfox

As far as using array.some([cb1,cb2,cb3]) does this produce the effect of if cb2 returns true it does not continue down the chain.(like e.stopPropogation()??) It doesn't seem to work this way currently so that may break stuff. (See first jsPanel.create https://github.com/Flyer53/jsPanel4/issues/196#issue-1400411872 example where it called all of the callbacks even though the second one returns true.

Please note that the way an Array of callbacks is processed is different depending on the used option:

Ok ... if you think the jsPanel.create(options, cb) callback is useful I'll leave it in there. But if I add support for multiple cb I most probably will do it the way regular option.callback is processed. Meaning either a single cb function or an array of callbacks where each cb is simply called one after the other (no Array.prototype.every() or Array.prototype.some()).

Let me think about this a bit ...

Flyer53 commented 2 years ago

@syonfox Ok ... I thought about this a bit more:

Now some of the extensions already use the second arg of jsPanel.create(options, cb) because that's what it is intended for. I could of course implement the use of multiple callbacks or an array of callbacks for extensions as well. But that would mean some extra code in the extensions without getting ny benefit. Remember, the regular option.callback is the place where you should add all the callbacks you might need for an individual panel.

Bottom line: Since I can't see a benefit I don't see a necessity to make any further changes concerning this topic.

syonfox commented 2 years ago

Makes seance to me. sorry for the distraction :) closing since I think that's how it works. bonus fun demo https://bsjs.sgol.pub/test.html