Skip to content
This repository has been archived by the owner on Sep 11, 2020. It is now read-only.

deleteScript can now be a callback #428

Merged
merged 2 commits into from
Oct 4, 2017
Merged

Conversation

derrekbertrand
Copy link
Contributor

@derrekbertrand derrekbertrand commented Jun 2, 2017

Not that anyone asked for it to my knowledge, but this is a handy option that I'm using on my server.

The signature of Image.deleteFile changed to take the jQuery element, instead of src. It now checks if your deleteScript option is a function, and if so passes the element to the callback allowing the end developer to get whatever info they need from the element and make their own AJAX call, if they desire.

The other delete options are ignored in this case, and it is up to the end developer to handle their AJAX options.


Use case: I'm using an API to handle my images that requires making a request like DELETE /api/v1/image/42, where 42 is the image's unique id. This is very difficult to do under the current structure. This modification allows you tons of freedom when deleting. If you want to use the old behavior, your code need not change.

I have never worked on a JS project, please be gentle.

@linkesch
Copy link
Owner

linkesch commented Jun 2, 2017

@derrekbertrand thanks. Good idea. I left you just one comment regarding backwards compatibility. After that's done, we can merge this.

@derrekbertrand
Copy link
Contributor Author

@orthes Where's the comment? I checked my branch's commit, and I don't see anything.

@linkesch
Copy link
Owner

linkesch commented Jun 3, 2017

@derrekbertrand it's here in the pull request.

Can you make Images.prototype.deleteFile function backward compatible for people using the current implementation? For example leave the first attribute as it was before (string) and add $file as a second attribute. And pass both also to deleteScript function for consistency.

@derrekbertrand
Copy link
Contributor Author

derrekbertrand commented Jun 3, 2017 via email

@derrekbertrand
Copy link
Contributor Author

derrekbertrand commented Jun 5, 2017

@orthes I believe this will do it. I'll see if I can find some docs to update.

EDIT:
I will make a similar edit to the upload script if it would be accepted. I'm using this API standard, btw.
Also, do I just edit the Wiki for docs?

EDIT2:
It looks like upload is plenty robust.

@linkesch linkesch merged commit d6ec905 into linkesch:master Oct 4, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants