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

Pass data arguments to modal? #531

Closed
andriijas opened this issue Nov 3, 2011 · 48 comments
Closed

Pass data arguments to modal? #531

andriijas opened this issue Nov 3, 2011 · 48 comments

Comments

@andriijas
Copy link
Contributor

Is this possible? https://gist.github.com/1336532

@fat
Copy link
Member

fat commented Nov 3, 2011

$('[data-controls-modal="countryModal"]').data().country

@fat fat closed this as completed Nov 3, 2011
@andriijas
Copy link
Contributor Author

That doesnt work when I have multiple lins using data-controls-modal="countryModal"

I know I can bind click event on my own but would just be insanely nice not to. :-)

@fat
Copy link
Member

fat commented Nov 4, 2011

ahh interesting... well I think for now we aren't going to pass the data options around for you, as i don't think this is a very common thing. sorry about that :(

@fl00r
Copy link

fl00r commented Mar 29, 2012

Common way is to use one modal for page.

Then, by clicking one of the links modal appears. So we should know which link shows modal to load information into it.

In my case I have got a row of items and a "show log" button for each. When I click "show log" modal appears but I need to understand what particular log to load.

@andriijas
Copy link
Contributor Author

@fl00r i agree it would be nice to know the origin of the show event, in the same way we know previous and next tab in tabbables. Im going to check the code and see if I can submit a pull request.

@jimryan
Copy link

jimryan commented Apr 1, 2012

+1 for this. Consider a case where multiple elements trigger the same modal with a slight variation in state (like a single attribute difference, or so).

@andriijas Have you made any progress on this?

@andriijas
Copy link
Contributor Author

Not yet. I Had an AFK weekend.

Sent from my frank phone, mhm

On Sunday 1 April 2012 at 21:20, Jim Ryan wrote:

+1 for this. Consider a case where multiple elements trigger the same modal with a slight variation in state (like a single attribute difference, or so).

@andriijas Have you made any progress on this?


Reply to this email directly or view it on GitHub:
#531 (comment)

@encodes1
Copy link

encodes1 commented Apr 3, 2012

i agree, perhaps having any attributes that start data-* be passed to the modal. would be nice if multiple modals are required.

@bjensen
Copy link

bjensen commented Apr 14, 2012

Good idea encodes

+1

@andriijas
Copy link
Contributor Author

I havent figured out a good way to implement this actually.

I would like to use the naming convention of tabs with relatedTarget, and the easy part is easy:

doing $target.modal(option, this) and $.fn.modal = function (option, relatedTarget) {

But then Im stuck - need to somehow jinx in the relatedTarget into the modal instance data, or pass it in as argument on each call, which I dont like.

There is also the thing that the modal plugin should probably still work without the data-api which makes it more complicated.

@encodes1
Copy link

i havn't wrote an implementation yet. but in my head, i feel that data-id='32' etc in the source code would be a good possible way of implementing it. would be easy and powerful, there would need to be other ways to set/get this data. but its a problem that can and im sure will be solved many ways

@andriijas
Copy link
Contributor Author

@encodes yeah problem is to pass the elem in somehow to the modal instance so that it can read the $(elem).data(), since the modal is instanciated once, and then reused the related target element from where the click originates needs to be passed in each time since the idea of this is that several links/sources could open the same modal.

(all this goes for collapsables as well)

@kxhawkins
Copy link

+1 for this. In my case, I want to use modals everywhere the user can input something. For instance, on my calendar, when the user clicks a day it opens an "Add event" modal with the date pre-populated.

@palamedes
Copy link

+1

1 similar comment
@danger2k7
Copy link

+1

@bronson
Copy link

bronson commented May 11, 2012

Big +1 for this. Right now I'm $('body').off('.data-api') and installing my own click handlers. Seems like a waste.

@bronson
Copy link

bronson commented May 11, 2012

Turns out this is actually pretty easy in 2.0.3. First, add your custom data as any number of data parameters.

<li><a data-person="Zim" data-toggle="modal" data-target="#person-modal"></a></li>
<li><a data-person="Dib" data-toggle="modal" data-target="#person-modal"></a></li>
...

Next, modify bootstrap-modal.js to update your modal's options every time a link is clicked. Otherwise, the modal always displays the data for the first person clicked.

diff --git a/app/assets/javascripts/bootstrap/bootstrap-modal.js b/app/assets/javascripts/bootstrap/bootstrap-modal.js
index 0465cb5..9228636 100644
--- a/app/assets/javascripts/bootstrap/bootstrap-modal.js
+++ b/app/assets/javascripts/bootstrap/bootstrap-modal.js
@@ -209,6 +209,9 @@
       var $this = $(this), href
         , $target = $($this.attr('data-target') || (href = $this.attr('href')) && href.replace(/.*(?=#[^\s]+$)/, '')) //strip for ie7
         , option = $target.data('modal') ? 'toggle' : $.extend({}, $target.data(), $this.data())
+      if(option == 'toggle') {
+        $.extend($target.data('modal').options, $this.data())
+      }

       e.preventDefault()
       $target.modal(option)

Finally, use the show event to stuff your custom information into the modal.

    $('#person-modal').on('show', function (event) {
        name = $(this).data('modal').options.person
        $(this).find('.person').html(name)
    })

If @fat can make this patch presentable and commit it, this feature req can be closed.

@d1rk
Copy link

d1rk commented May 11, 2012

yeah - would love to see that incorporated. It makes sense, breaks nothing and adds a lot of value to that component.

@cipater
Copy link

cipater commented May 17, 2012

+1

@Laurentvw
Copy link

+1!

@oferh
Copy link

oferh commented May 23, 2012

+1

1 similar comment
@scobal
Copy link

scobal commented Jun 3, 2012

+1

@bronson
Copy link

bronson commented Jun 6, 2012

Of course the constructor suffers the same problem. Here's a fix:

diff --git a/app/assets/javascripts/bootstrap/bootstrap-modal.js b/app/assets/javascripts/bootstrap/bootstrap-modal.js
index 0465cb5..a51813b 100644
--- a/app/assets/javascripts/bootstrap/bootstrap-modal.js
+++ b/app/assets/javascripts/bootstrap/bootstrap-modal.js
@@ -187,6 +187,7 @@
         , data = $this.data('modal')
         , options = $.extend({}, $.fn.modal.defaults, $this.data(), typeof option == 'object' && option)
       if (!data) $this.data('modal', (data = new Modal(this, options)))
+      else $.extend($this.data('modal').options, option)
       if (typeof option == 'string') data[option]()
       else if (options.show) data.show()
     })

Without this patch, modals only use the first options they were passed and ignore the ones passed to subsequent calls. With it, it uses the options as you'd expect.

Anyone willing to give this a code review? Hopefully it's time for a pull request.

@andriijas
Copy link
Contributor Author

@bronson please make a pull request and refer to this issue as it has been closed for 7 month but people are still actively discussing and contributing to it.

@datacarl
Copy link

+1

@kevinohashi
Copy link

@bronson I tested the code and it worked well. Thank you for that patch.

@milqmedia
Copy link

+2

1 similar comment
@antonheryanto
Copy link

+2

@tommyw
Copy link

tommyw commented Jul 11, 2012

+1, works well. Thanks

@alexandrejobin
Copy link

+1, great work!
i whish it will be included in the next version!!

@afanjul
Copy link

afanjul commented Jul 26, 2012

+3 for me!! this feature is a must for modal windows!!

@SummerDew
Copy link

+9001

kynan added a commit to checklisthq/bootstrap that referenced this issue Aug 2, 2012
Implements @bronson's suggestion, all credit to him.
kynan added a commit to checklisthq/bootstrap that referenced this issue Aug 14, 2012
Implements @bronson's suggestion, all credit to him.
@asacarter
Copy link

How do I set a data value programtically without using "data-" in the HTML?

Using $('selector').data('user', value) seems to get overwritten by the last click from a data attribute.

I am trying to pass a data value from one modal to another after a successful ajax request but the data value gets overwritten. The problem is the data could also come from a HTML link instead of the first modal.

$('#modal2').data('user_id', 5) // sets user_id to 5

$('#modal2').on('show', function (event) {
$('#modal2').data('user_id', $('#modal2').data('modal').options.user_id ); // sets user_id to something else
}

Link on SO to better explain it:

http://stackoverflow.com/questions/11992128/twitter-bootstrap-modal-issue-531

@bronson
Copy link

bronson commented Aug 27, 2012

@asacarter I do this: $('#my-modal').modal({person:'Gaz'}). If the modal is hidden, that will set modal.options.person and then call your show handler. (I think it toggles so, if it's showing, it gets hidden... fuzzy memory tho)

You need to apply the one-line constructor patch of course.

@FacioRatio
Copy link

I can't get the 'show' event callback to fire. (My modal is showing.) Do I need to put that code anywhere in particular?
Edit: nevermind, copy-paste error. Works great!

@akshaysmurthy
Copy link

+1

kynan added a commit to checklisthq/bootstrap that referenced this issue Sep 24, 2012
Implements @bronson's suggestion, all credit to him.
kynan added a commit to checklisthq/bootstrap that referenced this issue Dec 13, 2012
Implements @bronson's suggestion, all credit to him.
@niftylettuce
Copy link

👍

@leefaus
Copy link

leefaus commented May 29, 2013

FYI: Playing around with 3.0.
2.3.1
name = $(this).data('modal').options.person
3.0.0
name = $(this).data('bs.modal').options.person

Can anyone confirm this?

I also think the patch has not been applied because I am only seeing the first element clicked.

@asgeo1
Copy link

asgeo1 commented Jun 11, 2013

Agree with @leefaus, only data-* values from the first element clicked are available as options. Tested in v2.3.1

Bit disappointing... I thought some of the previous comments alluded to this having been fixed :(

@cvrebert
Copy link
Collaborator

#6825 is the most recent word on this. @fat is still "noodling".

@abrudtkuhl
Copy link

+1 Any progress on this?

Confirmed: Only the first element clicked passes data-* in v2.3.1

@alvincrespo
Copy link

+1

@jtal
Copy link

jtal commented Jun 26, 2013

Is there a fund somewhere that I can contribute to fix this > 2 year old bug? :>

@simonwh
Copy link

simonwh commented Jul 4, 2013

+1

1 similar comment
@danielsantiago
Copy link

+1

@jgc94131
Copy link

+1
I have to say this is a pretty obvious bug that needs to be fixed, and it greatly increases the utility of the modal widget and the ease by which it can be used. Of course the other option is to implement relatedTarget. Both have been suggested and progress is stalled.

@mpaf
Copy link

mpaf commented Oct 29, 2013

+1

@cvrebert
Copy link
Collaborator

Folks, this is addressed in v3.0.0+ thanks to event.relatedTarget getting implemented in dbed9da.

@twbs twbs locked and limited conversation to collaborators Jun 14, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests