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

API Provide a thin alternative to loadPanel/submitForm. #2352

Merged
merged 1 commit into from
Aug 22, 2013

Conversation

mateusz
Copy link
Contributor

@mateusz mateusz commented Aug 22, 2013

This is needed in some situations when we only want to update a
small single component, sometimes even using a different controller to
the one implied in the URL.

An example here is reloading dynamically the subsite dropdown without
reloading the entire page, updating a filter sidebar or suchlike.

@adrexia
Copy link
Contributor

adrexia commented Aug 22, 2013

👍

This is needed in some situations when we only want to update a
small single component, sometimes even using a different controller to
the one implied in the URL.

An example here is reloading dynamically the subsite dropdown without
reloading the entire page, updating a filter sidebar or suchlike.
@chillu
Copy link
Member

chillu commented Aug 22, 2013

Really well coded and documented, as usual - thanks Mateusz :)

chillu added a commit that referenced this pull request Aug 22, 2013
API Provide a thin alternative to loadPanel/submitForm.
@chillu chillu merged commit aa9403b into silverstripe:3.1 Aug 22, 2013
@stojg
Copy link

stojg commented Aug 22, 2013

:shipit:

@MarcusDalgren
Copy link
Contributor

Sorry for being late to the party here but is there a reason for not returning the xhr from the loadFragment() function?
You're kindof gimping the $.ajax functionality by not allowing the calling function to add .then() or .success() directly on the loadFragment() being made for doing some custom stuff.

@chillu
Copy link
Member

chillu commented Aug 22, 2013

Yeah we should uses promises more, Mateusz can you send fix+commit that? No need for a PR

@mateusz
Copy link
Contributor Author

mateusz commented Aug 22, 2013

There is no reason, apart from my own ignorance! Thanks @MarcusDalgren
137aa53

Now that I look at it, does it actually make sense to keep the loadfragmenterror trigger at all? I'm on a fence here: one the one hand this makes the code more cluttered, on the other it allows to write Entwine handlers which make a good job of preventing memory leaks.

@MarcusDalgren
Copy link
Contributor

I'm not an Entwine buff so I'm not sure in what contexts loadFragment() gets used.
When I made the comment I was thinking of the use case that the loadFragment() call comes from another Entwine element and you want to do stuff with the data when its returned or something. The most "natural" place for that code in some cases would be in the same context that made the loadFragment() call.

However you might want to setup some kind of generic handlers on the Entwine element which gets hit by the loadFragment() and in that case you use afterloadfragment and/or loadfragmenterror depending on your needs. The Entwine afterloadfragment/loadfragmenterror events will capture all loadFragment() calls made on that element while binding directly to the returned promise only affects that specific loadFragment() call.

At least that's my understanding of it.

@MarcusDalgren
Copy link
Contributor

What I'm trying to say is I'd keep both options just the way it is now.

@mateusz
Copy link
Contributor Author

mateusz commented Aug 22, 2013

Agreed then, leaving both in :-)

mateusz added a commit to mateusz/silverstripe-subsites that referenced this pull request Jul 8, 2014
Only to be merged after the
silverstripe/silverstripe-framework#2352 is
available, and only after Subsites 1.0.0 has been released.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants