GNOME Bugzilla – Bug 691253
prev/next page overlay buttons
Last modified: 2013-02-16 19:11:44 UTC
Now that we've switched away from using the continuous page view we need a better way of easily moving to the next/prev page. I think we can have click targets overlaid on the view. Visible slightly when hovering.
https://raw.github.com/gnome-design-team/gnome-mockups/master/documents/documents-preview-2page.png
Created attachment 236361 [details] [review] Add basic prev / next page overlay buttons
Review of attachment 236361 [details] [review]: Thanks for the patch! Looks mostly good, with some comments: ::: src/preview.js @@ +670,3 @@ + }, + + This seems to be unused right now @@ +680,3 @@ + _queueAutoHide: function() { + if (this._autoHideId != 0) + }, You should call unqueueAutoHide() here probably. @@ +718,3 @@ + }, + + }, I'd rather this function and _hide() to be called something more descriptive, like _fadeInButton/_fadeOutButton, for clarity (as we have show/hide already).
Created attachment 236366 [details] [review] Add basic prev / next page overlay buttons Don't hide buttons when hovering over them.
Created attachment 236367 [details] [review] Add basic prev / next page overlay buttons address comments
Review of attachment 236367 [details] [review]: Looks good to commit with these minor changes ::: src/preview.js @@ +646,3 @@ + _onEnterNotify: function() { + this._hover = true; + this.prev_widget.connect('clicked', Lang.bind(this, this._onPrevClicked)); Need to return false here, _onLeaveNotify() and _onMotion(). @@ +723,3 @@ + setModel: function(model) { + if (this._pageChangedId != 0) + } Should set this to zero, as we only reset pageChangedId when model != null below.
Attachment 236367 [details] pushed as bcb39fa - Add basic prev / next page overlay buttons