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

[ON HOLD] Propagate parameters from jQuery success/error/copmplete callbacks to onAfter[x] callbacks v2 #232

Closed
wants to merge 4 commits into from

Conversation

prantlf
Copy link
Contributor

@prantlf prantlf commented Mar 16, 2015

Additionally, call the onAfter[x] callbacks before the original jQuery ones. This may be a breaking change if somebody depended on the original jQuery callbacks executed earlier.

This suggest a resolution for the first half of the #225.

@jakerella
Copy link
Owner

I think I see the need for reordering the method calls here, but I want to get another pair of 👀 on it before merging. (And I'm totally fine with adding the original jQuery arguments.

@dcneiner, can you take a look? Note that this helps enable #233, although it is technically separate functionality.

@jakerella
Copy link
Owner

@dcneiner Any chance you can review this change soon?

@prantlf Looks like a conflict crept in from recent changes, but my guess is the conflict is small considering your changes. Can you resolve?

@dcneiner
Copy link
Collaborator

@jakerella Looking now!

@dcneiner
Copy link
Collaborator

Ok, first off, thank you @prantlf for all the thought and work you put into this feature. I think having the ability to pre-process the response seems like a great idea, and passing through the original jQuery arguments also is a great idea,

I do think keeping the names as onAfterSuccess but having it fire before the normal handler is a problem. This got me to thinking, it would be fairly straightforward to keep the existing onAfter{...} callbacks and add new onBefore callbacks for usage as you suggested.

If @jakerella agrees, would you be willing to try and tackle that change? Let us know if you need any help.

@dcneiner
Copy link
Collaborator

Oh, I forgot to mention – the conflicts seem to be just the built version of mockjax. So resolving it should be pretty easy.

@jakerella
Copy link
Owner

Hey @prantlf: any chance you can get to this PR (and maybe the other one) this week? I'm planning to release 2.0.0-beta this week/weekend and would love to include this. If not, we can plan for including it in 2.1.0, which shouldn't be too far behind.

@jakerella
Copy link
Owner

Last chance for 2.0.0... but we can always get it into 2.1.0.

@jakerella jakerella modified the milestones: Mockjax 2.0, 2.1.0 May 3, 2015
@jakerella jakerella changed the title Propagate parameters from jQuery success/error/copmplete callbacks to onAfter[x] callbacks v2 [ON HOLD] Propagate parameters from jQuery success/error/copmplete callbacks to onAfter[x] callbacks v2 Dec 20, 2015
@jakerella
Copy link
Owner

Placed on hold pending conversion to 2.x

@jakerella jakerella modified the milestones: 2.1.0, 2.1.1 Jan 23, 2016
@jakerella
Copy link
Owner

Closing this PR for inactivity. The issue is still there, and the code is on a good path. Any new PR would need to be merged into master at this point.

@jakerella jakerella closed this Jan 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants