Having a javascript problem

Having a javascript problem

by Mike Worth -
Number of replies: 4
At the request of a manager, I am trying to write a block that displays a searchable list of teachers. The list of teachers is fine but I am having problems with the JS (I'm a bit rusty)

This is what I've got so far:

function get_content() {

global $CFG, $USER, $COURSE;
$context_system = get_context_instance(CONTEXT_SYSTEM);

//use this to detect HOD/SSM
if (has_capability('moodle/site:viewreports', $context_system)) {

$this->content->text="<script type='text/javascript'>
function search(){
searchbox = document.getElementById('teacherlistsearch');
searchstring = searchbox.value;

teacherlist = document.getElementById('teacherlist');
teachers = teacherlist.childNodes;
alert(teachers.length);
for(i=0;teachers.length;i++){
//test.=teachers[i];
}
//alert(test);
}
</script>
<input type='test' onchange='search()' id='teacherlistsearch'><br><p id=teacherlist>";

$query="select id,firstname,lastname
from mdl_user
where (select count(*) from mdl_role_assignments where mdl_role_assignments.userid=mdl_user.id and mdl_role_assignments.roleid=3)>0
and deleted = 0
order by lastname
";

$teachers=get_records_sql($query);

foreach ($teachers as $teacher) {
$this->content->text .= "<a href='http://moodle.tauntons.ac.uk/course/user.php?id=1&mode=allgrade&user=$teacher->id'>$teacher->firstname $teacher->lastname</a><br>";
}

$this->context->text .="</p>";
return $this->content;

}
}

The teachers for loop causes it to take 100% of cpu until firefox decides it's not working and gives me the option to terminate it.

I'm using ff 2.0.0.17

Thanks,
Mike
Average of ratings: -
In reply to Mike Worth

Re: Having a javascript problem

by Tim Hunt -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers
First, please don't mix the JavaScript function definition in with the HTML, put in in an external file instead that you load with require_js.

Now to answer the question you asked. A more idiomatic way to iterate over the children of a particular element is:
for (var teacher = teacherlist.firstChild; teacher != null;
 teacher = teacher.nextSibling) {
 // Do stuff with teacher.
}
Average of ratings: -
In reply to Tim Hunt

Re: Having a javascript problem

by Mike Worth -
Thanks for the advice- I've now got teacherlist.js which is imported with require_js.

I have it almost there- my way of getting onload to work isn't perfect: I have a pair of body tags which are empty except from the onload attribute as I couldn't figure out a way to get it into the main body tag or get it to work with img or script tags.

My other problem is IEangry It complains that "line 6 char 5 error object doesn't support this property or method."

Here is a copy of teacherlist.js:

function search(){
searchbox = document.getElementById('teacherlistsearch');
searchstring = searchbox.value;

teacherlist = document.getElementById('teacherlist');
for (var teacher = teacherlist.firstChild; teacher != null; teacher = teacher.nextSibling) {
// Do stuff with teacher.
if((teacher.innerHTML.search(searchstring) == -1)||(searchstring =='')){teacher.style.display="none";}else{teacher.style.display="list-item";}
}
}


Thanks,
Mike

Average of ratings: -
In reply to Mike Worth

Re: Having a javascript problem

by Tim Hunt -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers

Actually, I wrote some very similar code to this recently. Have a look at the last function (filter()) in http://cvs.moodle.org/moodle/admin/roles/roles.js?view=markup

Specific things I would change:

  1. When you create a new variable, you should declare it with var, so the first line of your code would be better as var searchbox = document.getElementById('teacherlistsearch');, and similarly elsewhere.
  2. To get the content to search, innerText is probably better than innerHTML, except that it does not work cross-browser, so you have to do var textToSearch = capcell.innerText || capcell.textContent;
  3. I was using if (textToSearch.indexOf(searchstring) >= 0) instead of the search method - not sure why, but my way works even if searchstring is '' without a separate condition.
  4. I spent ages trying to work out why trying to set teacher.style.display="none" or teacher.style.display="list-item" did not work in IE (I was getting the same error as you). In the end I gave up and added .hidden {display: none;} to my style sheet, and then made the JavaScript change teacher.className. See the set_visible function, which is using YUI, which is probably overkill.
  5. Notice that I did not take my own advice about iterating through the DOM wink. Actually, one thing to watch out for with the firstChild/nextChild approach is that if you have whitespace in the HTML code, you get text nodes between the element nodes, and so you may need an if (teacher.nodeType == 1).
Average of ratings: -
In reply to Tim Hunt

Re: Having a javascript problem

by Mike Worth -
I got it working before I found this reply- seems that the issue was being lazy and not declaring variables with var. The none and list-item seem to work in ie for me once I use var.

This is what I have that works:

function search(){
searchbox = document.getElementById('teacherlistsearch');
searchstring= new RegExp(searchbox.value,'gi');
var teacherlist = document.getElementById('teacherlist');
for (var teacher = teacherlist.firstChild; teacher != null; teacher = teacher.nextSibling) {
// Do stuff with teacher.
if((teacher.innerHTML.search(searchstring) == -1)||(searchbox.value =='')){teacher.style.display="none";}else{teacher.style.display="list-item";}
}
}
Average of ratings: -