Fixing up the play/pause button logic for good!

src/js/background/model/player.js

@@ -38,6 +38,7 @@ define(function(require) {
                 iframePort: null,
                 buffers: [],
                 bufferType: '',
+                cueing: false,
 
                 //  Suffix alarm with unique identifier to prevent running after browser closed & re-opened.
                 //  http://stackoverflow.com/questions/14101569/chrome-extension-alarms-go-off-when-chrome-is-reopened-after-time-runs-out
@@ -97,6 +98,7 @@ define(function(require) {
                 if (playOnActivate || playerState === PlayerState.Playing || playerState === PlayerState.Buffering) {
                     this.get('youTubePlayer').loadVideoById(videoOptions);
                 } else {
+                    this.set('cueing', true);
                     this.get('youTubePlayer').cueVideoById(videoOptions);
                 }
 
@@ -206,7 +208,23 @@ define(function(require) {
 
         isPausable: function() {
             var state = this.get('state');
-            var isPausable = state === PlayerState.Playing || state === PlayerState.Buffering;
+            //  If the player is playing then it's obvious that it can be paused.
+            var isPausable = state === PlayerState.Playing;
+
+            //  However, if the player is buffering, then it's not so simple. The player might be buffering and paused/unstarted.
+            if (state === PlayerState.Buffering) {
+                var previousState = this.get('previousState');
+
+                //  When seeking it's even more complicated. The seek might result in the player beginning playback, or remaining paused.
+                var wasPlaying = previousState === PlayerState.Playing || previousState === PlayerState.Buffering;
+
+                if (this.get('seeking') && !wasPlaying) {
+                    isPausable = false;
+                } else {
+                    //  If the player is 'cueuing' a song then the user doesn't expect to see a flicker of buffering. Only when loading/playing.
+                    isPausable = !this.get('cueing');
+                }
+            }
 
             return isPausable;
         },
@@ -337,6 +355,10 @@ define(function(require) {
             var playerState = this._getPlayerState(youTubePlayerState);
             this.set('previousState', this.get('state'));
             this.set('state', playerState);
+
+            if (this.get('previousState') === PlayerState.Buffering) {
+                this.set('cueing', false);
+            }
         },
 
         _onYouTubePlayerChangeLoading: function(model, loading) {

src/js/background/model/searchResult.js

@@ -15,11 +15,6 @@
                 listItemType: ListItemType.SearchResult,
                 song: null
             };
-        },
-        //  TODO: Why is this needed?
-        //  SearchResults are never saved to the server.
-        sync: function() {
-            return false;
         }
     });
 

src/js/foreground/view/appBar/playPauseButtonView.js

@@ -41,7 +41,7 @@
         },
 
         onRender: function() {
-            this._setState(this.model.get('enabled'), this.player.get('state'));
+            this._setState(this.model.get('enabled'));
         },
 
         _onClick: function() {
@@ -56,33 +56,16 @@
             this._setState(enabled, this.player.get('state'));
         },
 
-        _onPlayerChangeState: function(model, state) {
-            this._setState(this.model.get('enabled'), state);
+        _onPlayerChangeState: function() {
+            this._setState(this.model.get('enabled'));
         },
 
-        _setState: function(enabled, playerState) {
+        _setState: function(enabled) {
             this.$el.toggleClass('is-disabled', !enabled);
 
-            //  If the player is playing then the pause icon clearly needs to be shown.
-            var showPauseIcon = playerState === PlayerState.Playing;
-
-            //  TODO: This still flickers when first loading the stream because there's a slight 'buffering' that isnt' user-initiated.
-            //  However, if the player is buffering, then it's not so simple. The player might be buffering and paused/unstarted.
-            if (playerState === PlayerState.Buffering) {
-                var previousState = this.player.get('previousState');
-
-                //  When seeking it's even more complicated. The seek might result in the player beginning playback, or remaining paused.
-                var wasPlaying = previousState === PlayerState.Playing || previousState === PlayerState.Buffering;
-
-                if (this.player.get('seeking') && !wasPlaying) {
-                    showPauseIcon = false;
-                } else {
-                    showPauseIcon = true;
-                }
-            }
-
-            this.ui.pauseIcon.toggleClass('is-hidden', !showPauseIcon);
-            this.ui.playIcon.toggleClass('is-hidden', showPauseIcon);
+            var isPausable = this.player.isPausable();
+            this.ui.pauseIcon.toggleClass('is-hidden', !isPausable);
+            this.ui.playIcon.toggleClass('is-hidden', isPausable);
         }
     });
 

src/js/foreground/view/behavior/tooltip.js

@@ -56,23 +56,23 @@ define(function() {
 
         _onMouseEnter: function() {
             //  Defer applying tooltips until absolutely necessary for rendering performance.
-            if (!this.isDecorated) {
-                this.isDecorated = true;
-
-                //  Wrap in a RAF to allow for :hover effects to take place which might affect whether textTooltipable overflows or not.
-                requestAnimationFrame(function() {
-                    this._setTooltips();
-
-                    //  Since calling toggle will force the tooltip to show -- wait the normal delay amount to emulate its effect.
-                    setTimeout(function() {
-                        if (!this.view.isDestroyed && this.$el.is(':hover')) {
-                            //  This forces a tooltip to appear immediately if it exists. This is necessary because decorating
-                            //  the element has been delayed until mouseenter for performance, but that is when tooltip rendering triggers, too
-                            this.$el.qtip('toggle', true);
-                        }
-                    }.bind(this), tooltipDelay);
-                }.bind(this));
-            }
+            //if (!this.isDecorated) {
+            //    this.isDecorated = true;
+
+            //    //  Wrap in a RAF to allow for :hover effects to take place which might affect whether textTooltipable overflows or not.
+            //    requestAnimationFrame(function() {
+            //        this._setTooltips();
+
+            //        //  Since calling toggle will force the tooltip to show -- wait the normal delay amount to emulate its effect.
+            //        setTimeout(function() {
+            //            if (!this.view.isDestroyed && this.$el.is(':hover')) {
+            //                //  This forces a tooltip to appear immediately if it exists. This is necessary because decorating
+            //                //  the element has been delayed until mouseenter for performance, but that is when tooltip rendering triggers, too
+            //                this.$el.qtip('toggle', true);
+            //            }
+            //        }.bind(this), tooltipDelay);
+            //    }.bind(this));
+            //}
         },
 
         _setTooltips: function() {

src/js/foreground/view/listItemButton/playPauseSongButtonView.js

@@ -62,9 +62,6 @@
 
         _setState: function() {
             var isPausable = this._isPausable();
-
-            //  TODO: Fix this along with the playPauseButtonView lgoic.
-            //  TODO: There's a difference between buffering-->play and buffering-->paused. Don't want to change button when buffering-->paused. How to tell the difference?
             this.ui.pauseIcon.toggleClass('is-hidden', !isPausable);
             this.ui.playIcon.toggleClass('is-hidden', isPausable);
         },