LGTM :+1:

I would remove the commented out code though.
On Apr 14, 2014 5:47 PM, "Dave St.Germain" <notifications@github.com> wrote:

> In lms/static/js/pdf-analytics.js:
>
> > +// this event is loaded after the others to accurately represent the order of events:
> > +// click next -> pagechange
> > +$(function() {
> > +  var first_page = true;
> > +  var curr_page = 1;
> > +
> > +  $(window).bind("pagechange", function(event) {
> > +    // log every page render
> > +    var page = event.originalEvent.pageNumber;
> > +    var old_page = PDFView.previousPageNumber;
> > +    // pagechange is called many times per viewing.
> > +    if (PDFView.previousPageNumber !== page || first_page) {
> > +      first_page = false;
> > +      // delay the event logging because otherwise it's possible that gotopage will appear before the
> > +      // navigation events (next/previous/scroll/etc). it's tricky to get this precisely right.
> > +      setTimeout(function(){
>
> @mulby <https://github.com/mulby> I've removed that setTimeout. In order
> to do that, I had to change the way scrolling is determined.
>
> I also renamed the search events as per your suggestions.
>
> --
> Reply to this email directly or view it on GitHub<https://github.com/edx/edx-platform/pull/3243/files#r11609997>
> .
>