From fa4cfa1269a510a364088ba70ae05b40d7dd74d7 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Fri, 7 Jun 2013 15:19:41 -0400 Subject: [PATCH] ScreenTrack refactor - removes logic from TopicView didInsertElement --- .../discourse/components/screen_track.js | 170 ++++++++++-------- .../discourse/controllers/topic_controller.js | 12 -- .../discourse/routes/topic_route.js | 5 + .../discourse/views/embedded_post_view.js | 7 +- .../javascripts/discourse/views/post_view.js | 23 +-- .../javascripts/discourse/views/topic_view.js | 69 +++---- 6 files changed, 140 insertions(+), 146 deletions(-) diff --git a/app/assets/javascripts/discourse/components/screen_track.js b/app/assets/javascripts/discourse/components/screen_track.js index 19c2173fe51..a635a8a7e73 100644 --- a/app/assets/javascripts/discourse/components/screen_track.js +++ b/app/assets/javascripts/discourse/components/screen_track.js @@ -8,99 +8,102 @@ **/ Discourse.ScreenTrack = Ember.Object.extend({ - // Don't send events if we haven't scrolled in a long time - PAUSE_UNLESS_SCROLLED: 1000 * 60 * 3, + init: function() { + var screenTrack = this; + this.reset(); + }, - // After 6 minutes stop tracking read position on post - MAX_TRACKING_TIME: 1000 * 60 * 6, + start: function(topicId) { + // Create an interval timer if we don't have one. + if (!this.get('interval')) { + var screenTrack = this; + this.set('interval', setInterval(function () { + screenTrack.tick(); + }, 1000)); + } - totalTimings: {}, + var currentTopicId = this.get('topicId'); + if (currentTopicId && (currentTopicId !== topicId)) { + this.flush(); + this.reset(); + } + this.set('topicId', topicId); + }, - // Elements to track - timings: {}, - topicTime: 0, - cancelled: false, + stop: function() { + this.flush(); + this.reset(); + this.set('topicId', null); + if (this.get('interval')) { + clearInterval(this.get('interval')); + this.set('interval', null); + } + }, track: function(elementId, postNumber) { - this.timings["#" + elementId] = { + this.get('timings')["#" + elementId] = { time: 0, postNumber: postNumber }; }, + stopTracking: function(elementId) { + delete this.get('timings')['#' + elementId]; + }, + // Reset our timers reset: function() { - this.lastTick = new Date().getTime(); - this.lastFlush = 0; - this.cancelled = false; - }, - - // Start tracking - start: function() { - var _this = this; - this.reset(); - this.lastScrolled = new Date().getTime(); - this.interval = setInterval(function() { - return _this.tick(); - }, 1000); - }, - - // Cancel and eject any tracking we have buffered - cancel: function() { - this.cancelled = true; - this.timings = {}; - this.topicTime = 0; - clearInterval(this.interval); - this.interval = null; - }, - - // Stop tracking and flush buffered read records - stop: function() { - clearInterval(this.interval); - this.interval = null; - return this.flush(); + this.set('lastTick', new Date().getTime()); + this.set('lastScrolled', new Date().getTime()); + this.set('lastFlush', 0); + this.set('cancelled', false); + this.set('timings', {}); + this.set('totalTimings', {}); + this.set('topicTime', 0); + this.set('cancelled', false); }, scrolled: function() { - this.lastScrolled = new Date().getTime(); + this.set('lastScrolled', new Date().getTime()); }, flush: function() { - var highestSeenByTopic, newTimings, topicId, - _this = this; - if (this.cancelled) { - return; - } + if (this.get('cancelled')) { return; } + // We don't log anything unless we're logged in if (!Discourse.User.current()) return; - newTimings = {}; - Object.values(this.timings, function(timing) { - if (!_this.totalTimings[timing.postNumber]) - _this.totalTimings[timing.postNumber] = 0; + var newTimings = {}; - if (timing.time > 0 && _this.totalTimings[timing.postNumber] < _this.MAX_TRACKING_TIME) { - _this.totalTimings[timing.postNumber] += timing.time; + // Update our total timings + var totalTimings = this.get('totalTimings'); + Object.values(this.get('timings'), function(timing) { + if (!totalTimings[timing.postNumber]) + totalTimings[timing.postNumber] = 0; + + if (timing.time > 0 && totalTimings[timing.postNumber] < Discourse.ScreenTrack.MAX_TRACKING_TIME) { + totalTimings[timing.postNumber] += timing.time; newTimings[timing.postNumber] = timing.time; } timing.time = 0; }); - topicId = this.get('topic_id'); + var topicId = parseInt(this.get('topicId'), 10); var highestSeen = 0; - $.each(newTimings, function(postNumber){ + Object.keys(newTimings, function(postNumber) { highestSeen = Math.max(highestSeen, parseInt(postNumber, 10)); }); - highestSeenByTopic = Discourse.get('highestSeenByTopic'); + var highestSeenByTopic = Discourse.get('highestSeenByTopic'); if ((highestSeenByTopic[topicId] || 0) < highestSeen) { highestSeenByTopic[topicId] = highestSeen; } if (!Object.isEmpty(newTimings)) { + Discourse.ajax('/topics/timings', { data: { timings: newTimings, - topic_time: this.topicTime, + topic_time: this.get('topicTime'), topic_id: topicId }, cache: false, @@ -109,37 +112,39 @@ Discourse.ScreenTrack = Ember.Object.extend({ 'X-SILENCE-LOGGER': 'true' } }); - this.topicTime = 0; + + this.set('topicTime', 0); } - this.lastFlush = 0; + this.set('lastFlush', 0); }, tick: function() { + // If the user hasn't scrolled the browser in a long time, stop tracking time read - var diff, docViewBottom, docViewTop, sinceScrolled, - _this = this; - sinceScrolled = new Date().getTime() - this.lastScrolled; - if (sinceScrolled > this.PAUSE_UNLESS_SCROLLED) { + var sinceScrolled = new Date().getTime() - this.get('lastScrolled'); + if (sinceScrolled > Discourse.ScreenTrack.PAUSE_UNLESS_SCROLLED) { this.reset(); return; } - diff = new Date().getTime() - this.lastTick; - this.lastFlush += diff; - this.lastTick = new Date().getTime(); - if (this.lastFlush > (Discourse.SiteSettings.flush_timings_secs * 1000)) { + + var diff = new Date().getTime() - this.get('lastTick'); + this.set('lastFlush', this.get('lastFlush') + diff); + this.set('lastTick', new Date().getTime()); + if (this.get('lastFlush') > (Discourse.SiteSettings.flush_timings_secs * 1000)) { this.flush(); } // Don't track timings if we're not in focus if (!Discourse.get("hasFocus")) return; - this.topicTime += diff; - docViewTop = $(window).scrollTop() + $('header').height(); - docViewBottom = docViewTop + $(window).height(); + this.set('topicTime', this.get('topicTime') + diff); + var docViewTop = $(window).scrollTop() + $('header').height(); + var docViewBottom = docViewTop + $(window).height(); - // TODO: Eyeline has a smarter more accurate function here - - return Object.keys(this.timings, function(id) { + // TODO: Eyeline has a smarter more accurate function here. It's bad to do jQuery + // in a model like component, so we should refactor this out later. + var screenTrack = this; + return Object.keys(this.get('timings'), function(id) { var $element, elemBottom, elemTop, timing; $element = $(id); if ($element.length === 1) { @@ -148,11 +153,32 @@ Discourse.ScreenTrack = Ember.Object.extend({ // If part of the element is on the screen, increase the counter if (((docViewTop <= elemTop && elemTop <= docViewBottom)) || ((docViewTop <= elemBottom && elemBottom <= docViewBottom))) { - timing = _this.timings[id]; + timing = screenTrack.timings[id]; timing.time = timing.time + diff; } } }); } +}); + + +Discourse.ScreenTrack.reopenClass({ + + // Don't send events if we haven't scrolled in a long time + PAUSE_UNLESS_SCROLLED: 1000 * 60 * 3, + + // After 6 minutes stop tracking read position on post + MAX_TRACKING_TIME: 1000 * 60 * 6, + + + /** + Returns a Screen Tracking singleton + **/ + instance: function() { + if (this.screenTrack) { return this.screenTrack; } + this.screenTrack = Discourse.ScreenTrack.create(); + return this.screenTrack; + } }); + diff --git a/app/assets/javascripts/discourse/controllers/topic_controller.js b/app/assets/javascripts/discourse/controllers/topic_controller.js index 9dbbd444512..00da7c209d4 100644 --- a/app/assets/javascripts/discourse/controllers/topic_controller.js +++ b/app/assets/javascripts/discourse/controllers/topic_controller.js @@ -293,18 +293,6 @@ Discourse.TopicController = Discourse.ObjectController.extend(Discourse.Selected this.get('content').convertArchetype('regular'); }, - startTracking: function() { - var screenTrack = Discourse.ScreenTrack.create({ topic_id: this.get('content.id') }); - screenTrack.start(); - this.set('content.screenTrack', screenTrack); - }, - - stopTracking: function() { - var screenTrack = this.get('content.screenTrack'); - if (screenTrack) screenTrack.stop(); - this.set('content.screenTrack', null); - }, - // Toggle the star on the topic toggleStar: function(e) { this.get('content').toggleStar(); diff --git a/app/assets/javascripts/discourse/routes/topic_route.js b/app/assets/javascripts/discourse/routes/topic_route.js index 17ae6cd67ab..b3122bfe767 100644 --- a/app/assets/javascripts/discourse/routes/topic_route.js +++ b/app/assets/javascripts/discourse/routes/topic_route.js @@ -92,6 +92,7 @@ Discourse.TopicRoute = Discourse.Route.extend({ topicController.set('multiSelect', false); this.controllerFor('composer').set('topic', null); + Discourse.ScreenTrack.instance().stop(); if (headerController = this.controllerFor('header')) { headerController.set('topic', null); @@ -103,7 +104,11 @@ Discourse.TopicRoute = Discourse.Route.extend({ controller.set('model', model); this.controllerFor('header').set('topic', model); this.controllerFor('composer').set('topic', model); + Discourse.TopicTrackingState.current().trackIncoming('all'); controller.subscribe(); + + // We reset screen tracking every time a topic is entered + Discourse.ScreenTrack.instance().start(model.get('id')); } }); diff --git a/app/assets/javascripts/discourse/views/embedded_post_view.js b/app/assets/javascripts/discourse/views/embedded_post_view.js index 765759e4c1b..a7cbc247290 100644 --- a/app/assets/javascripts/discourse/views/embedded_post_view.js +++ b/app/assets/javascripts/discourse/views/embedded_post_view.js @@ -11,8 +11,11 @@ Discourse.EmbeddedPostView = Discourse.View.extend({ classNames: ['reply'], didInsertElement: function() { - var postView = this.get('postView') || this.get('parentView.postView'); - postView.get('screenTrack').track(this.get('elementId'), this.get('post.post_number')); + Discourse.ScreenTrack.instance().track(this.get('elementId'), this.get('post.post_number')); + }, + + willDestroyElement: function() { + Discourse.ScreenTrack.instance().stopTracking(this.get('elementId')); } }); diff --git a/app/assets/javascripts/discourse/views/post_view.js b/app/assets/javascripts/discourse/views/post_view.js index 7cbf4e2528f..b3ddc34c768 100644 --- a/app/assets/javascripts/discourse/views/post_view.js +++ b/app/assets/javascripts/discourse/views/post_view.js @@ -17,17 +17,6 @@ Discourse.PostView = Discourse.View.extend({ 'parentPost:replies-above'], postBinding: 'content', - // TODO really we should do something cleaner here... this makes it work in debug but feels really messy - screenTrack: function() { - var parentView = this.get('parentView'); - var screenTrack = null; - while (parentView && !screenTrack) { - screenTrack = parentView.get('screenTrack'); - parentView = parentView.get('parentView'); - } - return screenTrack; - }.property('parentView'), - postTypeClass: function() { return this.get('post.post_type') === Discourse.Site.instance().get('post_types.moderator_action') ? 'moderator' : 'regular'; }.property('post.post_type'), @@ -213,7 +202,11 @@ Discourse.PostView = Discourse.View.extend({ }); }, - didInsertElement: function(e) { + willDestroyElement: function() { + Discourse.ScreenTrack.instance().stopTracking(this.$().prop('id')); + }, + + didInsertElement: function() { var $post = this.$(); var post = this.get('post'); var postNumber = post.get('scrollToAfterInsert'); @@ -233,10 +226,8 @@ Discourse.PostView = Discourse.View.extend({ } this.showLinkCounts(); - var screenTrack = this.get('screenTrack'); - if (screenTrack) { - screenTrack.track(this.$().prop('id'), this.get('post.post_number')); - } + // Track this post + Discourse.ScreenTrack.instance().track(this.$().prop('id'), this.get('post.post_number')); // Add syntax highlighting Discourse.SyntaxHighlighting.apply($post); diff --git a/app/assets/javascripts/discourse/views/topic_view.js b/app/assets/javascripts/discourse/views/topic_view.js index bfbd51f4dfb..ef300498cbf 100644 --- a/app/assets/javascripts/discourse/views/topic_view.js +++ b/app/assets/javascripts/discourse/views/topic_view.js @@ -48,8 +48,7 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { }.observes('progressPosition', 'topic.filtered_posts_count', 'topic.loaded'), updateTitle: function() { - var title; - title = this.get('topic.title'); + var title = this.get('topic.title'); if (title) return Discourse.set('title', title); }.observes('topic.loaded', 'topic.title'), @@ -95,19 +94,15 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { // This view is being removed. Shut down operations willDestroyElement: function() { - var screenTrack, controller; + this.unbindScrolling(); $(window).unbind('resize.discourse-on-scroll'); - controller = this.get('controller'); - controller.set('onPostRendered', null); + // Unbind link tracking + this.$().off('mouseup.discourse-redirect', '.cooked a, a.track-link'); - screenTrack = this.get('screenTrack'); - if (screenTrack) { - screenTrack.stop(); - } + this.get('controller').set('onPostRendered', null); - this.set('screenTrack', null); this.resetExamineDockCache(); // this happens after route exit, stuff could have trickled in @@ -115,8 +110,9 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { }, didInsertElement: function(e) { - var topicView = this; this.bindScrolling({debounce: 0}); + + var topicView = this; $(window).bind('resize.discourse-on-scroll', function() { topicView.updatePosition(false); }); var controller = this.get('controller'); @@ -124,19 +120,11 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { topicView.postsRendered.apply(topicView); }); - // Insert our screen tracker - var screenTrack = Discourse.ScreenTrack.create({ topic_id: this.get('topic.id') }); - screenTrack.start(); - this.set('screenTrack', screenTrack); - this.$().on('mouseup.discourse-redirect', '.cooked a, a.track-link', function(e) { return Discourse.ClickTrack.trackClick(e); }); this.updatePosition(true); - - // Watch all incoming topic changes - this.get('topicTrackingState').trackIncoming("all"); }, debounceLoadSuggested: Discourse.debounce(function(lookup){ @@ -177,8 +165,7 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { }, 50), resetRead: function(e) { - this.get('screenTrack').cancel(); - this.set('screenTrack', null); + Discourse.ScreenTrack.instance().reset(); this.get('controller').unsubscribe(); var topicView = this; @@ -205,11 +192,10 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { // Called for every post seen, returns the post number postSeen: function($post) { - var post, postNumber, screenTrack; - post = this.getPost($post); + var post = this.getPost($post); if (post) { - postNumber = post.get('post_number'); + var postNumber = post.get('post_number'); if (postNumber > (this.get('topic.last_read_post_number') || 0)) { this.set('topic.last_read_post_number', postNumber); } @@ -401,11 +387,8 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { this.set('progressPosition', post.get('index')); }, - nonUrgentPositionUpdate: Discourse.debounce(function(opts){ - var screenTrack = this.get('screenTrack'); - if(opts.userActive && screenTrack) { - screenTrack.scrolled(); - } + nonUrgentPositionUpdate: Discourse.debounce(function(opts) { + Discourse.ScreenTrack.instance().scrolled(); this.set('controller.currentPost', opts.currentPost); },500), @@ -414,16 +397,12 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { }, updatePosition: function(userActive) { - var $lastPost, firstLoaded, lastPostOffset, offset, - title, info, rows, screenTrack, _this, currentPost; - - _this = this; - rows = $('.topic-post'); + var rows = $('.topic-post'); if (!rows || rows.length === 0) { return; } - info = Discourse.Eyeline.analyze(rows); // if we have no rows + var info = Discourse.Eyeline.analyze(rows); if(!info) { return; } // top on screen @@ -432,8 +411,9 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { } // bottom of screen + var currentPost; if(info.bottom === rows.length-1) { - currentPost = _this.postSeen($(rows[info.bottom])); + currentPost = this.postSeen($(rows[info.bottom])); this.nextPage($(rows[info.bottom])); } @@ -441,8 +421,9 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { this.updateDock(Ember.View.views[rows[info.bottom].id]); // mark everything on screen read + var topicView = this; $.each(info.onScreen,function(){ - var seen = _this.postSeen($(rows[this])); + var seen = topicView.postSeen($(rows[this])); currentPost = currentPost || seen; }); @@ -461,10 +442,10 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { console.error("can't update position "); } - offset = window.pageYOffset || $('html').scrollTop(); - firstLoaded = this.get('firstPostLoaded'); + var offset = window.pageYOffset || $('html').scrollTop(); + var firstLoaded = this.get('firstPostLoaded'); if (!this.docAt) { - title = $('#topic-title'); + var title = $('#topic-title'); if (title && title.length === 1) { this.docAt = title.offset().top; } @@ -478,8 +459,8 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { } // there is a whole bunch of caching we could add here - $lastPost = $('.last-post'); - lastPostOffset = $lastPost.offset(); + var $lastPost = $('.last-post'); + var lastPostOffset = $lastPost.offset(); if (!lastPostOffset) return; if (offset >= (lastPostOffset.top + $lastPost.height()) - $(window).height()) { @@ -499,7 +480,7 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { return Discourse.TopicTrackingState.current(); }.property(), - browseMoreMessage: (function() { + browseMoreMessage: function() { var category, opts; opts = { @@ -534,7 +515,7 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { } else { return Ember.String.i18n("topic.read_more", opts); } - }).property('topicTrackingState.messageCount') + }.property('topicTrackingState.messageCount') });