Amber: Add support for others methods in data-method

[faustinoaq]

Description

Suggested by @Sija here: https://github.com/amberframework/amber/pull/715#discussion_r178404341

Additional Information

The idea is to use something like this:

document.querySelectorAll("a[data-method]").forEach(function (element) {
    var method = element.getAttribute("data-method");
    element.addEventListener("click", function (e) {
        e.preventDefault();
        var message = element.getAttribute("data-confirm") || "Are you sure?";
        if (confirm(message)) {
            var form = document.createElement("form");
            var input = document.createElement("input");
            form.setAttribute("action", element.getAttribute("href"));
            form.setAttribute("method", "POST");
            input.setAttribute("type", "hidden");
            input.setAttribute("name", "_method");
            input.setAttribute("value", method);
            form.appendChild(input);
            document.body.appendChild(form);
            form.submit();
        }
    });
});

^ That code will support multiples methods, allowing to use data-method in others a links

Demo here: https://jsfiddle.net/rgvsmhvy/3/ (updated)

[eliasjpr]

@faustinoaq the hidden input method in the example above is missing the name attribute.

document.querySelectorAll("a[data-method]").forEach(function (element) {
    var method = element.getAttribute("data-method");
    element.addEventListener("click", function (e) {
        e.preventDefault();
        var message = element.getAttribute("data-confirm") || "Are you sure?";
        if (confirm(message)) {
            var form = document.createElement("form");
            var input = document.createElement("input");
            form.setAttribute("action", element.getAttribute("href"));
            form.setAttribute("method", "POST");
            input.setAttribute("name", "_method");  // Input name expected by the framework
            input.setAttribute("type", "hidden");
            input.setAttribute("value", method);
            form.appendChild(input);
            document.body.appendChild(form);
            form.submit();
        }
    });
});

[faustinoaq]

@eliasjpr I updated the code, Thank you!

[eliasjpr]

I don't understand the issue. The suggestion here should already work

[eliasjpr]

@faustinoaq ^^

[faustinoaq]

Hi @eliasjpr I already fixed that in my local branch:

the idea is to allow writing things like:

a data-method="put" href="/some/put" Trigger a put method
a data-method="patch" href="/some/patch" Trigger a patch method
a data-method="post" href="/some/post" Trigger a post method

/cc @Sija

[eliasjpr]

@faustinoaq But this should be working, I do not think this is broken.

Also I am afraid that you might be addressing many issues. I think it would be good to quickly open a PR for this. I don't think is wise for it to be address in your current body of work

[Sija]

@eliasjpr nope, ATM it works only for data-method='delete', see below:

https://github.com/amberframework/amber/blob/bea1063f640e0c7466989fd50cd0453b812d7a39/assets/js/amber.js#L235-L253

[Sija]

@faustinoaq LGTM as an interim fix. IMO porting rails-ujs would be way to go in the long run though. I'd make a PR with that, yet I'm still short of time :(

[faustinoaq]

@Sija sure, no problem. I wil publish a new PR including this changes very soon.

I think in the future we would be glad use your rails-ujs port :wink:

[eliasjpr]

@Sija I see thanks, I got confuse for a moment but I do see the hard coded value DELETE. Open a PR fixing that and would be happy to review it promptly.

As per rails-ujs yes it seems like a good to have, but also fear that we might be making too much of a decision for the developer. I'm also kinda digging the minimalist approach of Stimulus https://github.com/stimulusjs/stimulus

[eliasjpr]

@faustinoaq I would recommend to open a separate PR for this issue. This fix should sit on its own PR.

There might be downsides to include it the body of work for the Error pages and Amber Watch.

[faustinoaq]

@eliasjpr No problem, in fact I won't open one PR but many of them, depending on what file was modified.

That's the reason Why I'm taking a bit more of time to publish them, I will do that this week , though :wink:

[eliasjpr]

@faustinoaq what I meant to say was that we can open a PR quickly for this issue and get it merge. This does not have to wait for your work on Amber Watch and Error Pages is what I meant. If you want any of us here can open a PR fixing this.

[faustinoaq]

Ok, no problem, I gonna publish this one today :wink: