Issue with confirmation modal in latest develop

[mkrecek234]

Steps to reproduce:

• You have a JsCallback opened by on click as a modal with confirmation:

$button->on('click', null, null, ['confirm' => 'Really delete?'])->atkAjaxec([
                'url' => $jsCallbackDelete
]);

Expected behaviour (as it was until just recently):

• jsCallbackDelete is called
• Modal is closed

Erroneous behaviour now:

• jsCallbackDelete is called
• Modal is not closed, JS error thrown:

Please fix @mvorisek

[mvorisek]

please provide complete repro code or advise (preferably, if the code can be much shorter) how to modify some existing demo that uses the components you use

[mvorisek]

thank you - also for the trace screenshot

also next time, please post here the code (or commit test to your fork, and then link it here), we do not want to use atk4 repos as a sandpit

[mvorisek]

The JS error is legit, $jsCallbackDelete (as named in the issue description) is rendered to jQuery with that selector.

Did this work in the past? If yes, when and what change has broken it?

(as per example in PR you opened) the fix is to newer worry about the URL but instead use the JsCallback as event directly

$b1 = new Button('Click me (error atkAjaxex');
$jsCallback = \Atk4\Ui\JsCallback::addTo($app)->set(function($jsCallback) {
        return [new \Atk4\Ui\JsToast('Executed. But not closed modal...')];
});

$b1->on('click', null, $jsCallback, ['confirm' => 'Really delete?']);

[mkrecek234]

@mvorisek You tell me what change it was ;-) Basically I realised it now, so must have been in the past 2 weeks probably. We should document such changes ideally. This worked without issues before mimicking the atkAjaxec calls in Table\Column\Delete as it was used prior.

Fix is working

[mvorisek]

thank you for the feedback - introduced in #1930 in

the change should be ok, but let's keep this issue open to investigate this issue more, mainly if we can present passing an jQuery selector into url (JS has no types), so the user can receive a better error.

[mvorisek]

I can confirm the original usage was always wrong. Here is what was rendered (on the latest develop, the same is rendered except a "better" context selector):

$('#atk_layout_maestro_button_2').on('click', function (event) {
    event.preventDefault();
    event.stopPropagation();
    $.atkConfirm({
        message: 'Really delete?',
        onApprove: function () {
            $(this).atkAjaxec({
                url: $(this).atkAjaxec({
                    url: 'button.php?__atk_cb_atk_layout_maestro_jscallback=ajax&__atk_cbtarget=atk_layout_maestro_jscallback',
                    urlOptions: [],
                    confirm: null,
                    apiConfig: null,
                    storeName: null,
                }),
            });
        },
        options: { button: { ok: 'Ok', cancel: 'Cancel' } },
        context: this,
    });
});

notice the URL is is another atkAjaxec call, due a coincidence, it worked originally as the 2nd atkAjaxec was ignored as the button (reffered with the original selector) had loading class - https://github.com/atk4/ui/blob/4.0.0/js/src/plugins/ajaxec.plugin.js#L18

in PHP all works correctly, in JS, there are no strong types, so impossible to prevent such issues in general, but I plan for several months already to unimplement JsExpressionable iface from View, which should detect such wrong usages in PHP directly (with nice early/PHP error)