GNOME Bugzilla – Bug 753199
Places dialog is showed when there is only one page
Last modified: 2015-08-31 18:56:18 UTC
The bookmark list isn't necessary if there is only one page. I think that this was the intent of the code, because it does: this._bookmarkPage.enabled = hasMultiplePages && this._bookmarks; this._showPlaces.enabled = hasMultiplePages; the problem is that this._showPlaces wasn't the action name, but was the function name.
Created attachment 308703 [details] [review] preview: don't show places dialog with only one page
Created attachment 308704 [details] [review] places: revert commit 5da94839422a70d0ce20e4073e074ebbd1147525
Review of attachment 308704 [details] [review]: This isn't necessary because we have cases when we don't have a index. So only the bookmark list is showed.
Review of attachment 308703 [details] [review]: Thanks for catching this, Alessandro. One small nitpick: ::: src/preview.js @@ +139,3 @@ })); + this._showPlaces = Application.application.lookup_action('places'); + let showPlacesId = this._showPlaces.connect('activate', Lang.bind(this, this._showPlacesDialog)); I prefer to call the variable for the GAction 'this._places', and keep this._showPlaces as the method name. The other similar variables are named after the corresponding GAction names, so this would be consistent.
Created attachment 309608 [details] [review] preview: don't show places dialog with only one page https://bugzilla.gnome.org/show_bug.cgi?id=753199#c4
Created attachment 309610 [details] [review] preview: don't show places dialog with only one page
Review of attachment 309610 [details] [review]: Perfect. I added pointers to the original commits that introduced the typo before pushing.