ansaurus

Question

Answer 1

+1 A:

I see you keep appending a div to the document with id=popup and a lot of other stuff with hardcoded ID. In HTML IDs must be unique throughout the whole document and a particular ID (eg 'popup') must only appear once. What happens when two elements share the same ID is undefined and the browser can return anything the browser's developer feel like.

So your query $('#popup:last') does not do what you think it means.

In cases like this regular DOM methods work much better than jQuery-ism:

// shortcut. I hate typing document...
function newElement (tag, spec) {
    var el = document.createElement(tag);
    for (var n in spec) {
        el[n] = spec[n];
    }
    return el;
} 

function popup_AskYN(msg,yes_fn,no_fn){
    // because we get the references directly we don't need to
    // assign ids and therefore avoid id collisions:

    var popup = $(newElement('div')).append($(newElement('p')).append(msg));
    var yes_button = $(newElement('a',{href:'#'})).append('Yes!');
    var no_button = $(newElement('a',{href:'#'})).append('No.');

    popup.append(yes_button).append(no_button);
    popup.center();

    yes_button.click(yes_fn);
    no_button.click(no_fn);

    yes_button.click(function(){
        popup.fadeOut('fast').remove();
    });
    no_button.click(function(){
        popup.fadeOut('fast').remove();
    });

    $('body').append(popup);
}

slebetman 2010-01-09 12:59:20

edited: showing you that you don't need ids.

slebetman 2010-01-09 13:15:23

can not get your example to work.. sorry. new to this..

Prodigga 2010-01-09 13:38:28

Ahh... sorry, forgot to add `$('body').append(popup)`.

slebetman 2010-01-09 15:47:06

Answer 2

A:

You can create a unique identifier using the number of existing popups.

var popupId =  $("#popup").size();

Then create the new popup using the id and reference it for the buttons as well.

$('body').append("<div id='" + popupId + "'><p>"+msg+"</p><a id='popup_ok' href='#'>Ok.</a></div>");

Sam Tyson 2010-01-09 13:01:13

but i've got popup's being created and deleted. so the number of them changes.

Prodigga 2010-01-09 13:07:47

Store and increment a value then...var id = $("body").data(popupID);$("body").data(popupID, id++);

Sam Tyson 2010-01-09 13:16:41

i've added what i've now got to my first post. buttons still stuff up when there is more than one popup. what it appears to be doing is using the current value of popup_AskYNId for the selectors whenever i click a button. i know this because when i have 2 popups, popup 2 works fine, but not 1. when i have 5, 5 works fine, but not the others.

Prodigga 2010-01-09 13:44:00

Answer 3

+1 A:

This works fine for me if I simply change all the IDs to classes. Centering the popups absolutely on the screen doesn't lend itself to having multiple instances of a popup, though, if that's what you're after. I had to comment out the centering/positioning code to see that it was working.

I would also recommend that when someone clicks the yes or no links in a popup, the answer replaces the original popup's content instead of creating a new popup. I see that you're attempting to have the question fade out before the answer appears, but notice that in your current implementation, the original popup question is removed before the animation finishes, so there's no advantage to removing the question and creating a new popup window with the answer as opposed to just replacing the content.

If you want the question to fade out before the answer appears, one option would be to only remove the question popup after the animation completes, which you can do with fadeOut's second parameter, which is a callback to be executed when the animation completes. Again, this doesn't lend itself to having multiple instances of popup questions, though. The reason is because the answer popup is disassociated from the question popup with your method of removing the question and appending the answer to the body. This would also be solved by simply replacing the question with the answer. If you still want the same fade effect, you can fade the popup out, then change its contents, then fade it back in.

Here is a version of your code that works and changes it as I suggest:

<html>
<head>
  <script type="text/JavaScript" src="http://code.jquery.com/jquery.js"&gt;&lt;/script&gt;
  <script type="text/JavaScript">
    $(function() {
      $('a').click(function(e) {
        e.preventDefault();

        var num1 = Math.floor(Math.random()*11),
        num2 = Math.floor(Math.random()*11);

        popup_AskYN(
          "Want me to tell you what " + num1 + " + " + num2 + " is?",
          "It's " + (num1 + num2) + ", silly!",
          "I didn't want to, anyway!"
        );
      });

      $('.popup_ok').live('click', function(e) {
        e.preventDefault();

        $(this).closest('.popup').fadeOut('fast', function() {
          $(this).remove();
        });
      });
    });

    function popup_AskYN(msg, yes, no){
      $('body').append("<div class='popup'><p>"+msg+"</p><a class='popup_yes' href='#'>Yes!</a><a href='#' class='popup_no'>No.</a></div>");
      var yes_button = $('.popup_yes:last');
      var no_button = $('.popup_no:last');
      var popup = $('.popup:last');

      yes_button.click(function() {
        popup.html('<p>' + yes + '<a class="popup_ok" href="#">Ok.</a>');
      });

      no_button.click(function() {
        popup.html('<p>' + no + '<a class="popup_ok" href="#">Ok.</a>');
      });
    }
  </script>

  <style type="text/css">
  .popup {
    border:1px solid black;
    margin-bottom: 10px;
  }
  .popup a {
    margin:10px;
  }
  </style>
</head>

<body>
  <a href="#">Launch the popup!</a>
</body>

Jimmy Cuadra 2010-01-09 14:11:39

how comes the ID does not work? what am i doing wrong?

Prodigga 2010-01-09 21:56:04

i just tried it, and the same problem persists. i printed out the ID of the popup into the popup, stopped centering and made it position:relative so they get placed in different rows. regardless of whichever button i press, the last popup and only the last one will ever disapear.

Prodigga 2010-01-09 21:59:58

I added code to my original answer to show you what I mean. Using IDs doesn't work because, as slebetman noted, each ID must be unique. As it turns out, there is no need for IDs here so it works fine just be changing them to classes.

Jimmy Cuadra 2010-01-10 00:33:18

well i still cant figure out what i was doing wrong. heh. the solution here is what i'm looking for. had to modify it a little bit so that it would still accept functions being passed for yes and no. i've added the fixed version of this to my post for whoever wants it. thanks mate

Prodigga 2010-01-10 09:27:07

the reason i wanted to pass functions is because im going to use the popups for more than just displaying an answer like this. what you suggested makes sense (and makes the script more efficient if this was its sole purpose) but i plan on using this for other stuff too. what for, im not too sure yet. but its always good to keep your options open i guess. besides, its good fun.

Prodigga 2010-01-10 09:30:10

sigh - after fiddling with it further, i still get the error. in your example, the strings are concentrated and then sent to the function. we know the value of num1 + num2 now. however, what i do is for the yes function, i put in $('body').append('its '+num3); (Where num3 is just num1+num2, calculated inside this fn). so when i make a new popup without answering yes or no to the first, the values of num1 and num2 change. and then when i go to click yes on the first popup, it is using the values of the new num1 and num2, rather than the old ones..

Prodigga 2010-01-10 10:20:58

thats the problem i thought i had at the start (thats what i ment by going out of scope etc...?). not sure how to prevent this.. i figured the num1 and num2 vars would just be read as the values i had called them as.

Prodigga 2010-01-10 10:21:31

Add your new code to your original question and I'll take a look.

Jimmy Cuadra 2010-01-10 12:12:48

ansaurus

tags:

views:

answers:

Variables going beyond scope?

related questions