patternjavascriptMinor

Interupt redirect to catch potential newsletter signup

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

interuptcatchnewsletterredirectsignuppotential

Problem

This is a piece of code I am working on to reuse in a few places on various websites. It is used to capture name and email addresses for people interesting in registering for something, we want to add them a newsletter...So here is the scripts/CSS/HTML and it feels clunky. And maybe others might have some input on how I can do this better.

The Javascript (jQuery):

$(document).ready(function () {
$("img").each(function (i) {
    var imgUrla = $(this).closest("a").attr('href');
    $("a img").attr('data-link', imgUrla).attr('id', 'target');
    $(this).closest("a").contents().unwrap();
});
$('img#target').click(function (e) {
    var imgUrl = $(this).attr('data-link');
    $('#noThanks').attr('onclick', 'opentab("' + imgUrl + '");');
    $('#thanks').attr('value', imgUrl);
    var posTop = 200;
    $('body').append('');
    $('#achi').wrap('').removeClass('newHidden');
    $('#lightbox').css('top', posTop + 'px');
    $('#overlay').fadeTo(500, 0.75, function () {
        $('#lightbox').fadeTo(250, 1);
    });
});
$('#overlay, #lightbox_close').live('click', function (e) {
    $('#achi').unwrap('').addClass('newHidden');
    $('#overlay').remove();
    $('#lightbox_close').remove();
});
$("#submit").click(function () {
    $('form').submit(function () {
        var faults = $('input').filter(function () {
            return $(this).data('required') && $(this).val() === "";
        }).css("background-color", "pink");
        if (faults.length) return false;
    });
});
});

The code above does a few things. It looks for any image with hyperlinks and strips the tag then moves the href portions (the URL) into the image as a parameter called data-link. The next thing it does is when the image is clicked, it presents a floating div/form to capture the name and email of the user with cancel and submit. If they hit submit and the fields are not filled then it errors (it is a simple validation), if the user hits cancel then it shoots them to the redirect url

Solution

This is a piece of code I am working on to reuse in a few places on various websites

This line alone tells me that it's a drop-in script. And that alone makes me tell you to skip jQuery altogether. Never assume a website uses jQuery. Also, not all systems can run jQuery. It could be due to compatibility with another library, an older version of jQuery or license restrictions. Who knows?

If you want to keep this script versatile (and sneaky):

Skip jQuery, create your own abstractions. This keeps your script flexible yet light.

Ship a custom build of jQuery, with only the things you need. Not the entire library.

Drop-in scripts should be as simple as adding a ` for the user. Use a "trojan-style" resource loading script instead. Basically it's a single script that loads all other stuff required. This is how Google scripts (API, Analytics) work. Resources include JS, CSS, and even HTML.




  
The CSS above was very specific and somewhat difficult. I had to get help with it because the overlay and form were not behaving right.

It's what you call defensive programming. What prevents the user from clobbering your styles if you have selectors such as

#overlay, img, #lightbox

. These names are too simple, and the user's stylesheets might contain similarly named selectors. Their styles will clobber your styles, even override them.

Keep your selectors specific, and namespaced. for instance:


  text


If you styled span like

span{color:red} and the user's style has span{color:green} after yours, then your styles are overridden. What you do is add more selectors to keep it specific, like #MyModalDialog span{color:red}

.

Also, by namespacing your style to

#MyModalDialog, it keeps your styles focused on what's inside the #MyModalDialog` and avoid clobbering the user's styles as well.

As for other stuff:

You don't declare handlers like this

$('#noThanks').attr('onclick', 'opentab("' + imgUrl + '");');

Do this instead:

$('#noThanks').on('click',function(){
  opentab(imgUrl);
});

Avoid applying CSS using JS

.css("background-color", "pink");

Instead, add a class that contains that style. Saves you debugging time looking for where pink is from.

.addClass('bg-pink');

bg-pink{
  background-color:pink;
}

Code Snippets

<div id="MyModalDialog">
  <span>text</span>
</div>

$('#noThanks').attr('onclick', 'opentab("' + imgUrl + '");');

$('#noThanks').on('click',function(){
  opentab(imgUrl);
});

.css("background-color", "pink");

.addClass('bg-pink');

bg-pink{
  background-color:pink;
}

Context

StackExchange Code Review Q#46695, answer score: 4

Revisions (0)

No revisions yet.