Could I get a code review for my query dialog/lightbox code please?

Fairly new with jQuery, and am looking for a critique of my code. I am not entirely happy with a few things I have done.

1. Using an id to uniquely identify the buttons, and appending the "_dialog" string to it, so I can target the blocks of code i want to make visible. Seems a bit brittle, but I can't think of a better way.

2. The duplication when binding the hide functions.

3. General feeling that the code is not very modular/clean.

I just want to know how I can improve my jQuery coding.

Thanks

HTML

 
  <a href="#" class="dialog" id="login">login</a>

Javascript

 
  function Dialog() {
  

      // Event handler for a dialog click.
      $('a.dialog').click(function () {
  

          var dialog = $("#" + this.id + "_dialog");
  

          // Bind the hide functions here, as the elements have now been loaded into the DOM.
          $("#overlay, #close").click(function () {
              HideDialog(dialog)
          });
          window.onresize = function () {
              CentreBoxInViewport(dialog);
          };
  

          SetupDialog(dialog);
          return false;
      });
  

      // Event handler for an image click.
      $('div.status > a').click(function () {
  

          // Get the object for the lightbox.
          var dialog = $("#lightbox");
  

          // Get the path to the image.
          var path = this.getAttribute("href");
  

          // Bind the hide functions here, as the elements have now been loaded into the DOM.
          $("#overlay, #close").click(function () {
              HideDialog(dialog)
          });
          window.onresize = function () {
              CentreBoxInViewport(dialog);
          };
  

          SetupLightBox(path, dialog);
          return false;
      });
  }
  

  function SetupLightBox(path, dialog) {
  

      // Create a new image.
      var image = new Image();
  

      // The onload function must come before we set the image's src, see link for explanation.
      // http://www.thefutureoftheweb.com/blog/image-onload-isnt-being-called.
  

      // Anonymous function set to onload event, to make sure we only proceed if the image has been loaded.
      image.onload = function () {
          $('#image').attr('src', path)
          CentreBoxInViewport(dialog);
          ShowOverlay(dialog);
      };
  

      // Set the src, and show the image.
      image.src = path;
  }
  

  function SetupDialog(dialog) {
      CentreBoxInViewport(dialog);
      ShowOverlay(dialog);
  }
  

  function ShowDialog(dialog) {
      dialog.fadeTo(200, 1);
  }
  

  function HideDialog(dialog) {
      dialog.fadeTo(200, 0, function () {
          dialog.hide();
          HideOverlay();
      });
  }
  

  function ShowOverlay(dialog) {
      $('#overlay').fadeTo(200, 0.5, function () {
          ShowDialog(dialog)
      });
  }
  

  function HideOverlay() {
      $('#overlay').fadeTo(200, 0, function () {
          $('#overlay').hide();
      });
  }
  

  function CentreBoxInViewport(dialog) {
  

      // Get the dimensions of the viewport.
      var height = $(window).height()
      var width = $(window).width();
  

      // Calculate the position of the lightbox.
      var boxtop = (height / 2) - dialog.height() / 2;
      var boxleft = (width / 2) - dialog.width() / 2;
  

      // Position the box.
      dialog.css({
          top: boxtop,
          left: boxleft
      });
  }

Topic Participants
dave.cavell

Could I get a code review for my query dialog/lightbox code please?

Could I get a code review for my query dialog/lightbox code please?

Topic Participants

dave.cavell