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

Deprecate the remote option of modals? #14034

Merged
merged 3 commits into from
Jul 6, 2014
Merged

Conversation

cvrebert
Copy link
Collaborator

@cvrebert cvrebert commented Jul 3, 2014

  • @fat closed possible implementation to address #13183 #13788 (which would have added the completely-reasonable ability to disable unwanted caching for remote)
  • remote doesn't currently give any "loading..." (or similar) visual indication while the network request is in-progress, and doesn't fire any "loading started" event
  • remote doesn't currently give any visual indication or fire any event when the network request fails
  • Depending on the use-case, the user may want to either reuse or refresh the modal's header/footer. In older versions, the user had no nice way of reloading the header. In newer versions, users who don't want to modify the header must regenerate it on their backend anyway. (See Add option called 'remotePlacement' for modals #13597)
  • The existence of remote can discourage the use of client-side templating, even when it might be a preferable option.

So, I think we have 3 options:

  1. Add several features to remote modals.
  2. (This PR.) Deprecate remote modals.
  3. Status quo: Intentionally leave remote modals in a somewhat half-assed state.

CC: @fat @twbs/team for discussion

@cvrebert cvrebert added this to the v3.2.1 milestone Jul 3, 2014
@cvrebert cvrebert changed the title Deprecate the remote option of modals Deprecate the remote option of modals? Jul 3, 2014
@hnrch02
Copy link
Collaborator

hnrch02 commented Jul 3, 2014

I'll also leave some feedback: I think this is the right way to go. As was stated previously, the remote option is indeed in a "half-assed" state. I'm thinking though, maybe we should mention jQuery#load as a starting point in the docs to help people transition away from using this option?

@cvrebert
Copy link
Collaborator Author

cvrebert commented Jul 3, 2014

Yeah, that'd probably be good. I'd want to also add some link about client-side templating.

@fat
Copy link
Member

fat commented Jul 6, 2014

yep, this sounds good to me. I think both ideas about giving people jumping off points if they want to go that route sound good 👍

cvrebert added a commit that referenced this pull request Jul 6, 2014
Deprecate the `remote` option of modals?
@cvrebert cvrebert merged commit 7933a50 into master Jul 6, 2014
@cvrebert cvrebert deleted the deprecate-modal-remote branch July 6, 2014 06:48
@cvrebert cvrebert mentioned this pull request Jul 6, 2014
@mbrodala
Copy link
Contributor

mbrodala commented Jul 7, 2014

@hnrch02 How about jQuery.fn.load instead? OTOH only people with a slight understanding of jQuery's internals will then understand that this is a prototype method.

@hnrch02
Copy link
Collaborator

hnrch02 commented Jul 7, 2014

@mbrodala I adopted jQuery#function some time ago, but I'm fine with either one. It's not that big of a deal, really.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants