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

AJAX framework handle redirect response side-effect #5555

Closed
adrenth opened this issue Apr 8, 2021 · 1 comment
Closed

AJAX framework handle redirect response side-effect #5555

adrenth opened this issue Apr 8, 2021 · 1 comment

Comments

@adrenth
Copy link
Contributor

adrenth commented Apr 8, 2021

  • OctoberCMS Build: 1.1.1+
  • PHP Version: 7.4
  • Database Engine: MySQL
  • Plugins Installed: not applicable

Description:

This is a follow up on this issue:

The addition of this specific code has nasty side-effects:

handleRedirectResponse: function(url) {
    window.location.assign(url)
    // Indicate that the AJAX request is finished if we're still on the current page
    // so that the loading indicator for redirects that cause a browser to download 
    // a file instead of leave the page will properly stop.
    // @see https://github.com/octobercms/october/issues/5055
    $el.trigger('ajaxDone')
},

In quite a lot of our projects we have a following situation as such:

<form data-handler="onPlaceOrder">
    <button type="submit" data-attach-loading>Place Order Now!</button>
</form>
public function onPlaceOrder(): RedirectResponse
{
    $this->placeOrder(...);

    return redirect()->to('/thank-you');
}

Now when the Place Order Now! is clicked, the button will be disabled (because of the data-attach-loading) while processing the request.

When a redirect response has been detected, it will point the browser to the new URL and immediately after that it will trigger the ajaxDone and de disabled-state of all form elements (which have data-attach-loading) will be discarded.

Which enables the user to press the button Place Order Now! again during the redirection process. Which results in a form which is submissed twice (or more).

For now, I have fixed this behavior in my projects using this code:

$(document).on('ajaxSetup', function (event, context) {
    context.options.handleRedirectResponse = function (url) {
        window.location.assign(url);
    }
});

The user who reported the issue with the file download response could have done this:

$(document).on('ajaxSetup', function (event, context) {
    if (context.handler === 'myComponent::downloadFile') {
        context.options.handleRedirectResponse = function (url) {
            window.location.assign(url);
            $(event.target).trigger('ajaxDone');
        }
    }
});
daftspunk added a commit that referenced this issue Apr 8, 2021
Repurposing this event should not happen. Framework is not concerned with the needs of loading indicators

Refs #5555
@daftspunk
Copy link
Member

Addressed in 99a0861

Thanks @adrenth

LukeTowers added a commit to wintercms/winter that referenced this issue Apr 9, 2021
…ct response via AJAX.

Fixes: octobercms/october#5555

Tested by wintercms/wn-test-plugin@9fb25e2

Refs:
- octobercms/october#5055 (original issue attempted to solve with previous code)
- octobercms/october#2780 (original issue regarding the hash change redirect not closing the loading indicator)
- octobercms/october@99a0861 (October's solution to the issue)
LukeTowers pushed a commit to wintercms/winter that referenced this issue Apr 9, 2021
…ator (#70)

Fix issue with premature hiding of loading indicators during a redirect response via AJAX.

Fixes: octobercms/october#5555

Tested by wintercms/wn-test-plugin@9fb25e2

Refs:
- octobercms/october#5055 (original issue attempted to solve with previous code)
- octobercms/october#5321 (solution to octobercms/october#5055)
- octobercms/october#2780 (original issue regarding the hash change redirect not closing the loading indicator)
- octobercms/october@99a0861 (October's solution to the issue)
LukeTowers pushed a commit to wintercms/wn-system-module that referenced this issue Apr 9, 2021
…ator (#70)

Fix issue with premature hiding of loading indicators during a redirect response via AJAX.

Fixes: octobercms/october#5555

Tested by wintercms/wn-test-plugin@9fb25e2

Refs:
- octobercms/october#5055 (original issue attempted to solve with previous code)
- octobercms/october#5321 (solution to octobercms/october#5055)
- octobercms/october#2780 (original issue regarding the hash change redirect not closing the loading indicator)
- octobercms/october@99a0861 (October's solution to the issue)
LukeTowers added a commit to wintercms/wn-system-module that referenced this issue Apr 26, 2021
…ct response via AJAX.

Fixes: octobercms/october#5555

Tested by wintercms/wn-test-plugin@9fb25e2

Refs:
- octobercms/october#5055 (original issue attempted to solve with previous code)
- octobercms/october#2780 (original issue regarding the hash change redirect not closing the loading indicator)
- octobercms/october@99a0861 (October's solution to the issue)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants