aspnet / jquery-ajax-unobtrusive

[Maintenance mode: only security and critical bug fixes are being worked on] Add-on to jQuery Ajax to enable unobtrusive options in data-* attributes.
Other
144 stars 113 forks source link

Support formaction attribute #40

Open nprieto95 opened 6 years ago

nprieto95 commented 6 years ago

This is an issue in support of the PR #36 . I hereby present a concrete case in which my personal experience with the product would improve if the functionality shown in that PR was merged.

Currently, when having a form like this one:

<form action="/Management/Products/Edit/A001"
    data-ajax="true"
    data-ajax-method="Post"
    data-ajax-mode="replace"
    data-ajax-update="#preview-container"
    id="form0" method="post">
    <div class="row">
        <section class="col-md-9">
            <h2>Edit</h2>
            <div class="form-row">
                <div class="form-group col-md-12">
                    <label for="Name">Name</label>
                    <input class="text-box single-line" id="Name" name="Name" type="text" value="Test" />
                </div>
            </div>
        </section>
        <section class="col-md-3">
            <h2>Preview</h2>
            <div id="preview-container">
                <product-card code="A001" name="Test"></product-card>
            </div>
        </section>
    </div>
    <a class="btn btn-dark" href="/Management/Products">Cancel</a>
    <button type="submit" class="btn btn-secondary" formaction="/Management/Products/Preview">
        Preview
    </button>
    <button type="submit" class="btn btn-success">
        Save
    </button>
</form>

clicking on the "Preview" button causes a request to be isssued to /Management/Products/Edit/A001 instead of /Management/Products/Preview . The second case would be desired as that is the destination that would be hit if we were using synchronous, old-style forms without unobtrusive jquery-ajax support.

Supporting formaction natively would eliminate the need to sometimes come up with workarounds to use a library that is really good in every other aspect I had the chance to encounter.

mkArtakMSFT commented 6 years ago

Thanks for contacting us, @nprieto95. @kichalla, I was under the impression that even today this is possible, wasn't it?

kichalla commented 6 years ago

@nprieto95 Could you share a standalone repro project that can represent the issue you are seeing?

mkArtakMSFT commented 6 years ago

Hi. We're closing this issue as no response or updates have been provided in a timely manner and we have been unable to reproduce it. If you have more details and are encountering this issue please add a new reply and re-open the issue.

kakone commented 6 years ago

@nprieto95 You can reopen this issue. The problem is very simple, the formaction attribute on button or input is not taken into account : http://plnkr.co/edit/lc82fluYOGguSHIeKJs2?p=preview When you click on the "Submit to another page" button, the formaction value ("another_action.html") should be called. It's not the case. The PR #36 corrects this.

mkArtakMSFT commented 6 years ago

@javiercn. can you please verify this and review the PR? Thanks!

drml commented 5 years ago

Any progress on this? The PR #36 fixing the issue is literally just one line of code, if I understand.

kakone commented 5 years ago

Any progress on this? The PR #36 fixing the issue is literally just one line of code, if I understand.

We will celebrate soon the 1 year of this PR of one line of code. Too bad there was not 5 minutes in 1 year to merge this PR :cry:

vanillajonathan commented 5 years ago

@mkArtakMSFT, @javiercn, @ryanbrandenburg, ping!

Stormgail commented 4 years ago

Just wanted to add that fixing this would be a big help for my team

kakone commented 4 years ago

Just wanted to add that fixing this would be a big help for my team

I think you should update the line of code by yourself and not use the npm package anymore. You must just replace the line url: this.action by url: clickTarget && clickTarget.attr('formaction') || this.action. Don't expect the pull request to be merged, we've been waiting for it a year and a half.

Stormgail commented 4 years ago

Just wanted to add that fixing this would be a big help for my team

I think you should update the line of code by yourself and not use the npm package anymore. You must just replace the line url: this.action by url: clickTarget && clickTarget.attr('formaction') || this.action. Don't expect the pull request to be merged, we've been waiting for it a year and a half.

Thanks, I did exactly that but thought it wouldn't hurt to at least mention it'd be useful

kakone commented 4 years ago

I now also added the support of the formmethod attribute in the PR #36.

alden-m commented 4 years ago

That's insane, two years for one line of code :)

akifyanbak commented 4 years ago

That's insane, two years for one line of code :)

Lol 😂

vanillajonathan commented 4 years ago

@mkArtakMSFT, @javiercn, @ryanbrandenburg, @terrajobst ping!

nprieto95 commented 4 years ago

Let's celebrate the anniversary this July 26th

kakone commented 4 years ago

Let's celebrate the anniversary this July 26th

We are getting closer

jumpingjackson commented 4 years ago

@mkArtakMSFT, @javiercn, @ryanbrandenburg, @terrajobst ping!

alden-m commented 4 years ago

Let's join, celebrate the anniversary, chit-chat, and get to know each other!

Alden Menz is inviting you to a scheduled Zoom meeting.

Topic: Celebrate ajax unobtrusive feature request anniversary Time: Jul 25, 2020, 02:00 PM Eastern Time (US and Canada)

Join Zoom Meeting https://us04web.zoom.us/j/74857196510?pwd=SmJoSWtJbjYrdlAwck5GamhDR1NFUT09

Meeting ID: 748 5719 6510 Password: 9PWJWg

vanillajonathan commented 4 years ago

@mkArtakMSFT, @javiercn, @ryanbrandenburg, @terrajobst ping! @DamianEdwards, @kichalla, @ajaybhargavb, @Eilon ping!

alden-m commented 4 years ago

After five minutes it is :d let's join and have some fun

Eilon commented 4 years ago

Tagging @Pilchie who is the current owner of this project. Unfortunately I don't work on this project anymore so I don't have much to add to this discussion.

kakone commented 4 years ago

@Pilchie, after two years of waiting, can you take 2 seconds (no need more) to merge the pull request #36. Thanks in advance.

kakone commented 4 years ago

Okay, Houston, we have a problem here !

mkalinski93 commented 3 years ago

Just wanted to add that fixing this would be a big help for my team

I think you should update the line of code by yourself and not use the npm package anymore. You must just replace the line url: this.action by url: clickTarget && clickTarget.attr('formaction') || this.action. Don't expect the pull request to be merged, we've been waiting for it a year and a half.

Thanks for the hint, I was heavily looking for it :D

jumpingjackson commented 3 years ago

@mkArtakMSFT, @javiercn, @ryanbrandenburg, @terrajobst ping!

I think its ridiculous that this simple 1 line of code fix cannot be be merged and published after 2+ years. Bueller?? Bueller?? Do we need to send some angry tweets?

kakone commented 3 years ago

IMPORTANT: This repository is in maintenance mode. We do not work, nor plan to work on any new features. Only security and critical bug fixes will be worked on.

I think it's a stupid thing. This little library is essential when you develop in ASP.Net Core, and it doesn't cost much to include one PR per year. I will have to offer another NPM package to support the formaction attribute :worried:

vanillajonathan commented 3 years ago

IMPORTANT: This repository is in maintenance mode. We do not work, nor plan to work on any new features. Only security and critical bug fixes will be worked on.

I think it's a stupid thing. This little library is essential when you develop in ASP.Net Core, and it doesn't cost much to include one PR per year. I will have to offer another NPM package to support the formaction attribute 😟

Yeah @mkArtakMSFT! 😡