Completely redo loading of notes with JS

Riyad Preukschas
1 parent 14164017
Showing 5 changed files with 134 additions and 100 deletions Show diff stats
app/assets/javascripts/note.js
app/contexts/notes/load_context.rb
app/helpers/notes_helper.rb
app/views/notes/_create_common.js.haml
app/views/notes/_load.js.haml
@@ -4,9 +4,8 @@ var NoteList = {
   target_params: null,
   target_id: 0,
   target_type: null,
-  first_id: 0,
-  last_id: 0,
-  disable:false,
+  bottom_id: 0,
+  loading_more_disabled: false,
   init:
     function(tid, tt, path) {
@@ -15,26 +14,23 @@ var NoteList = {
       this.target_type = tt;
       this.target_params = "&target_type=" + this.target_type + "&target_id=" + this.target_id;
-      // get notes
+      // get initial set of notes
       this.getContent();
-      // get new notes every n seconds
-      this.initRefresh();
-
       $('.delete-note').live('ajax:success', function() {
         $(this).closest('li').fadeOut(); });
-      $(".note-form-holder").live("ajax:before", function(){
+      $(".note-form-holder").on("ajax:before", function(){
         $(".submit_note").disable()
       })
-      $(".note-form-holder").live("ajax:complete", function(){
+      $(".note-form-holder").on("ajax:complete", function(){
         $(".submit_note").enable()
       })
       disableButtonIfEmptyField(".note-text", ".submit_note");
-      $(".note-text").live("focus", function(){
+      $(".note-text").on("focus", function(){
         $(this).css("height", "80px");
         $('.note_advanced_opts').show();
       });
@@ -44,64 +40,20 @@ var NoteList = {
         var filename = val.replace(/^.*[\\\/]/, '');
         $(".file_name").text(filename);
       });
-
     },
   /**
-   * Load new notes to fresh list called 'new_notes_list': 
-   * - Replace 'new_notes_list' with new list every n seconds
-   * - Append new notes to this list after submit
+   * Handle loading the initial set of notes.
+   * And set up loading more notes when scrolling to the bottom of the page.
    */
-  initRefresh:
-    function() {
-      // init timer
-      var intNew = setInterval("NoteList.getNew()", 10000);
-    },
-
-  replace:
-    function(html) {
-      $("#new_notes_list").html(html);
-    },
-
-  prepend:
-    function(id, html) {
-      if(id != this.last_id) {
-        $("#new_notes_list").prepend(html);
-      }
-    },
-
-  getNew:
-    function() {
-      // refersh notes list
-      $.ajax({
-        type: "GET",
-      url: this.notes_path,
-      data: "last_id=" + this.last_id + this.target_params,
-      dataType: "script"});
-    },
-
-  refresh:
-    function() {
-      // refersh notes list
-      $.ajax({
-        type: "GET",
-      url: this.notes_path,
-      data: "first_id=" + this.first_id + "&last_id=" + this.last_id + this.target_params,
-      dataType: "script"});
-    },
-
   /**
-   * Init load of notes: 
-   * 1. Get content with ajax call
-   * 2. Set content of notes list with loaded one
+   * Gets an inital set of notes.
    */
-
-
-  getContent: 
-    function() { 
+  getContent:
+    function() {
       $.ajax({
         type: "GET",
       url: this.notes_path,
@@ -111,10 +63,13 @@ var NoteList = {
       dataType: "script"});
     },
+  /**
+   * Called in response to getContent().
+   * Replaces the content of #notes-list with the given html.
+   */
   setContent:
-    function(fid, lid, html) {
-      this.last_id = lid;
-      this.first_id = fid;
+    function(last_id, html) {
+      this.bottom_id = last_id;
       $("#notes-list").html(html);
       // Init infinite scrolling
@@ -123,54 +78,126 @@ var NoteList = {
   /**
-   * Paging for old notes when scroll to bottom: 
-   * 1. Init scroll events with 'initLoadMore'
-   * 2. Load onlder notes with 'getOld' method
-   * 3. append old notes to bottom of list with 'append'
+   * Handle loading more notes when scrolling to the bottom of the page.
+   * The id of the last note in the list is in this.bottom_id.
    *
+   * Set up refreshing only new notes after all notes have been loaded.
    */
-  getOld:
+
+
+  /**
+   * Initializes loading more notes when scrolling to the bottom of the page.
+   */
+  initLoadMore:
     function() {
+      $(document).endlessScroll({
+        bottomPixels: 400,
+        fireDelay: 1000,
+        fireOnce:true,
+        ceaseFire: function() {
+          return NoteList.loading_more_disabled;
+        },
+        callback: function(i) {
+          NoteList.getMore();
+        }
+      });
+    },
+
+  /**
+   * Gets an additional set of notes.
+   */
+  getMore:
+    function() {
+      // only load more notes if there are no "new" notes
       $('.loading').show();
       $.ajax({
         type: "GET",
         url: this.notes_path,
-        data: "first_id=" + this.first_id + this.target_params,
+        data: "loading_more=1&after_id=" + this.bottom_id + this.target_params,
         complete: function(){ $('.notes-status').removeClass("loading")},
         beforeSend: function() { $('.notes-status').addClass("loading") },
         dataType: "script"});
     },
-  append:
+  /**
+   * Called in response to getMore().
+   * Append notes to #notes-list.
+   */
+  appendMoreNotes:
     function(id, html) {
-      if(this.first_id == id) { 
-        this.disable = true;
-      } else { 
-        this.first_id = id;
+      if(id != this.bottom_id) {
+        this.bottom_id = id;
         $("#notes-list").append(html);
       }
     },
-  initLoadMore:
+  /**
+   * Called in response to getMore().
+   * Disables loading more notes when scrolling to the bottom of the page.
+   * Initalizes refreshing new notes.
+   */
+  finishedLoadingMore:
     function() {
-      $(document).endlessScroll({
-        bottomPixels: 400,
-      fireDelay: 1000,
-      fireOnce:true,
-      ceaseFire: function() { 
-        return NoteList.disable;
-      },
-      callback: function(i) {
-        NoteList.getOld();
+      this.loading_more_disabled = true;
+
+      // from now on only get new notes
+      this.initRefreshNew();
+    },
+
+
+  /**
+   * Handle refreshing and adding of new notes.
+   *
+   * New notes are all notes that are created after the site has been loaded.
+   * The "old" notes are in #notes-list the "new" ones will be in #new-notes-list.
+   * The id of the last "old" note is in this.bottom_id.
+   */
+
+
+  /**
+   * Initializes getting new notes every n seconds.
+   */
+  initRefreshNew:
+    function() {
+      setInterval("NoteList.getNew()", 10000);
+    },
+
+  /**
+   * Gets the new set of notes (i.e. all notes after ).
+   */
+  getNew:
+    function() {
+      $.ajax({
+        type: "GET",
+      url: this.notes_path,
+      data: "loading_new=1&after_id=" + this.bottom_id + this.target_params,
+      dataType: "script"});
+    },
+
+  /**
+   * Called in response to getNew().
+   * Replaces the content of #new-notes-list with the given html.
+   */
+  replaceNewNotes:
+    function(html) {
+      $("#new-notes-list").html(html);
+    },
+
+  /**
+   * Adds a single note to #new-notes-list.
+   */
+  appendNewNote:
+    function(id, html) {
+      if(id != this.bottom_id) {
+        $("#new-notes-list").append(html);
       }
-      });
     }
 };
-var PerLineNotes = { 
+var PerLineNotes = {
   init:
     function() {
-      $(".line_note_link, .line_note_reply_link").live("click", function(e) {
+      $(".line_note_link, .line_note_reply_link").on("click", function(e) {
         var form = $(".per_line_form");
         $(this).closest("tr").after(form);
         form.find("#note_line_code").val($(this).attr("line_code"));
@@ -3,8 +3,7 @@ module Notes
     def execute
       target_type = params[:target_type]
       target_id   = params[:target_id]
-      first_id    = params[:first_id]
-      last_id     = params[:last_id]
+      after_id    = params[:after_id]
       @notes = case target_type
@@ -23,10 +22,8 @@ module Notes
                  project.wikis.reverse.map {|w| w.notes.fresh }.flatten[0..20]
                end
-      @notes = if last_id
-                 @notes.where("id < ?", last_id)
-               elsif first_id
-                 @notes.where("id > ?", first_id)
+      @notes = if after_id
+                 @notes.where("id > ?", after_id)
                else 
                  @notes
                end
@@ -0,0 +1,9 @@
+module NotesHelper
+  def loading_more_notes?
+    params[:loading_more].present?
+  end
+
+  def loading_new_notes?
+    params[:loading_new].present?
+  end
+end
@@ -5,7 +5,8 @@
     $('.note-form-holder #preview-link').text('Preview');
     $('.note-form-holder #preview-note').hide();
     $('.note-form-holder').show();
-    NoteList.prepend(#{note.id}, "#{escape_javascript(render partial: "notes/show", locals: {note: note})}");
+    NoteList.appendNewNote(#{note.id}, "#{escape_javascript(render "notes/show", note: note)}");
+
 - else
   :plain
     $(".note-form-holder").replaceWith("#{escape_javascript(render('form'))}");
 - unless @notes.blank?
-  - if params[:last_id]
+  - if loading_more_notes?
     :plain
-      NoteList.replace("#{escape_javascript(render(partial: 'notes/notes_list'))}");
+      NoteList.appendMoreNotes(#{@notes.last.id}, "#{escape_javascript(render 'notes/notes_list')}");
-  - elsif params[:first_id]
+  - elsif loading_new_notes?
     :plain
-      NoteList.append(#{@notes.last.id}, "#{escape_javascript(render(partial: 'notes/notes_list'))}");
+      NoteList.replaceNewNotes("#{escape_javascript(render 'notes/notes_list')}");
   - else
     :plain
-      NoteList.setContent(#{@notes.last.id}, #{@notes.first.id}, "#{escape_javascript(render(partial: 'notes/notes_list'))}");
+      NoteList.setContent(#{@notes.last.id}, "#{escape_javascript(render 'notes/notes_list')}");
 - else
-  - if params[:first_id]
+  - if loading_more_notes?
     :plain
-      NoteList.append(#{params[:first_id]}, "");
+      NoteList.finishedLoadingMore();
@@ -0,0 +1,9 @@		@@ -0,0 +1,9 @@
	1	+module NotesHelper
	2	+ def loading_more_notes?
	3	+ params[:loading_more].present?
	4	+ end
	5	+
	6	+ def loading_new_notes?
	7	+ params[:loading_new].present?
	8	+ end
	9	+end
1	- unless @notes.blank?	1	- unless @notes.blank?
2	- - if params[:last_id]	2	+ - if loading_more_notes?
3	:plain	3	:plain
4	- NoteList.replace("#{escape_javascript(render(partial: 'notes/notes_list'))}");	4	+ NoteList.appendMoreNotes(#{@notes.last.id}, "#{escape_javascript(render 'notes/notes_list')}");
5		5
6	- - elsif params[:first_id]	6	+ - elsif loading_new_notes?
7	:plain	7	:plain
8	- NoteList.append(#{@notes.last.id}, "#{escape_javascript(render(partial: 'notes/notes_list'))}");	8	+ NoteList.replaceNewNotes("#{escape_javascript(render 'notes/notes_list')}");
9		9
10	- else	10	- else
11	:plain	11	:plain
12	- NoteList.setContent(#{@notes.last.id}, #{@notes.first.id}, "#{escape_javascript(render(partial: 'notes/notes_list'))}");	12	+ NoteList.setContent(#{@notes.last.id}, "#{escape_javascript(render 'notes/notes_list')}");
13		13
14	- else	14	- else
15	- - if params[:first_id]	15	+ - if loading_more_notes?
16	:plain	16	:plain
17	- NoteList.append(#{params[:first_id]}, "");	17	+ NoteList.finishedLoadingMore();