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

Make .placeholder classname configurable #39

Closed
wants to merge 6 commits into from
Closed
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions jquery.placeholder.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
;(function(window, document, $) {

var isInputSupported = 'placeholder' in document.createElement('input'),
isTextareaSupported = 'placeholder' in document.createElement('textarea');
isTextareaSupported = 'placeholder' in document.createElement('textarea'),
placeholderClassName = 'placeholder';
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line isn’t needed, is it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used in clearPlaceholder and setPlaceholder functions, plus the domready and unload event handlers.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I was thinking is that this should be added to the code after $.fn.placeholder is declared:

$.fn.placeholder.className = 'placeholder';

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, so I should only get the className via $.fn.placeholder.className instead of a variable (outside of the function scopes)?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, and cache/re-use the $.fn.placeholder.className wherever possible (per scope).

We should probably move the$(…) and the $(window).bind(…) code inside of the $.fn.placeholder = fn block, while still making sure these only get called/bound once (even when $().placeholder() is used multiple times). We could use .one() instead of .bind(), but what about the $(…)?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use .one() for the ready event.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tested this and does it work?

This doesn’t seem to work: http://jsfiddle.net/mathias/tMDrZ/ Any ideas? Perhaps we could use something like this: http://jsfiddle.net/tMDrZ/1/

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, it doesn't work. I thought about it, and ultimately we can assume that by the time .placeholder is called, domready is already fired.


if (isInputSupported && isTextareaSupported) {

Expand All @@ -15,6 +16,7 @@
} else {

$.fn.placeholder = function() {
placeholderClassName = $.fn.placeholder.className || placeholderClassName;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just var className = $.fn.placeholder.className;, no?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, it’s not needed/used in this scope, so I guess you can remove this line.

return this.filter((isInputSupported ? 'textarea' : ':input') + '[placeholder]')
.bind('focus.placeholder', clearPlaceholder)
.bind('blur.placeholder', setPlaceholder)
Expand All @@ -28,7 +30,7 @@
// Look for forms
$('form').bind('submit.placeholder', function() {
// Clear the placeholder values so they don’t get submitted
var $inputs = $('.placeholder', this).each(clearPlaceholder);
var $inputs = $('.' + placeholderClassName, this).each(clearPlaceholder);
setTimeout(function() {
$inputs.each(setPlaceholder);
}, 10);
Expand All @@ -37,7 +39,7 @@

// Clear placeholder values upon page reload
$(window).bind('unload.placeholder', function() {
$('.placeholder').val('');
$('.' + placeholderClassName).val('');
});

}
Expand All @@ -56,11 +58,11 @@

function clearPlaceholder() {
var $input = $(this);
if ($input.val() === $input.attr('placeholder') && $input.hasClass('placeholder')) {
if ($input.val() === $input.attr('placeholder') && $input.hasClass(placeholderClassName)) {
if ($input.data('placeholder-password')) {
$input.hide().next().show().focus().attr('id', $input.removeAttr('id').data('placeholder-id'));
} else {
$input.val('').removeClass('placeholder');
$input.val('').removeClass(placeholderClassName);
}
}
}
Expand Down Expand Up @@ -91,9 +93,9 @@
}
$input = $input.removeAttr('id').hide().prev().attr('id', id).show();
}
$input.addClass('placeholder').val($input.attr('placeholder'));
$input.addClass(placeholderClassName).val($input.attr('placeholder'));
} else {
$input.removeClass('placeholder');
$input.removeClass(placeholderClassName);
}
}

Expand Down