function showBlock(element_id, element) {
dropdown_tracker = element_id //using global variable to track dropdown
if(document.getElementById(element_id).style.display == "block") {
outsideOf = null;
$(document.body).descendants().invoke('stopObserving','click',outside); //stopObserving the outside request as we have finished
document.getElementById(element_id).style.display = "none";
} else {
outsideOf = element_id;
document.getElementById(element_id).style.display = "block";
$(document.body).descendants().invoke('observe','click',outside);
element.stopObserving('click',outside); //need to then stop this from observing the outside - ie requires it at least
}
};
function outside(event) {
element = event.element();
alert(element.id)
if( outsideOf == null ) { //this shouldnt happen, but just incase
$(document.body).descendants().invoke('stopObserving','click',outside);
return;
}
element = Event.element(event);
if( element.descendantOf( $(outsideOf) ) !== true) {
outsideOf = null;
$(document.body).descendants().invoke('stopObserving','click',outside);
document.getElementById(dropdown_tracker).style.display = "none";
}
};
Refactorings
No refactoring yet !
Tj Holowaychuk
May 19, 2009, May 19, 2009 14:39, permalink
No rating.
Login to rate!
You are using lots of global variables, and your generic functions should be wrapped in an anonymous function call to preserve scope or namespaced
ttdavett.myopenid.com
May 19, 2009, May 19, 2009 18:57, permalink
No rating.
Login to rate!
Yea, I have already started to try and create a class using prototype to gid rid of all these global variables. I'm not sure if I'm going about this right though, how does this look so far?
var DropDown = Class.create({
initialize: function(date, appt_counter, times, edit_or_add) {
start_drop_id = "start_drop_" + date + "_" + appt_counter;
start_time_box_id = "start_time_box_" + date + "_" + appt_counter;
end_drop_id = "start_drop_" + date + "_" + appt_counter;
end_time_box_id = "end_time_box_" + date + "_" + appt_counter;
times_array = times;
switch (edit_or_add) {
case 'add': selected_or_not = false; break;
case 'edit': selected_or_not = true; break;
default: selected_or_not = false;
}
this.observe_for_dropdowns();
},
observe_for_dropdowns: function() {
$(start_time_box_id).observe('click',function(event){ //start_time_id is dynamically defined as a global variable above
Event.stop(event);
element = event.element();
var start_drop_id = element.up().down('.start_dropdown').id;
this.show_block(start_drop_id, element);
}.bind(this))
},
show_block: function(element_id, element) {
if(document.getElementById(element_id).style.display == "block") {
outsideOf = null;
$(document.body).descendants().invoke('stopObserving','click',outside); //stopObserving the outside request as we have finished
document.getElementById(element_id).style.display = "none";
} else {
outsideOf = element_id;
document.getElementById(element_id).style.display = "block";
$(document.body).descendants().invoke('observe','click',outside);
element.stopObserving('click',outside); //need to then stop this from observing the outside - ie requires it at least
};
}, ...
Tj Holowaychuk
May 20, 2009, May 20, 2009 00:18, permalink
No rating.
Login to rate!
just put var infront of the variable name to limit its scope like below. Im not a fan of prototype so I cant confirm, but you could still clean up alot
function foo() {
bar = 'something' // This is global
var bar2 = 'something' // This is not
}
// For example these are effectively the same.. and I would not name variables in the manor your using, just use 'operation', 'op', etc
switch (edit_or_add) {
case 'add': selected_or_not = false; break;
case 'edit': selected_or_not = true; break;
default: selected_or_not = false;
}
// Same as above
selected = op == 'edit'
The following is the code I am using to show a dropdown select menu that consists of times for a calendar. When you click on the box, a div element is displayed that looks like a html select element. If you click outside of the pulldown when it is open, it closes. I need to create this custom select dropdown menu so that I can customize the content(i.e. add alerts for certain times). I am using a global variable to pass the id of the dropdown element to a function that detects if the click is outside of the element. It seems like it's generally a bad idea to use global variables like this. Is there a way to pass the dropdown id to the outside function? This code is also pretty slow in IE, so if you see anything that can be improved, please let me know.