GNOME Bugzilla – Bug 603078
wrong sizing of 'all workspace' windows in overview
Last modified: 2010-02-19 18:37:07 UTC
Created attachment 148548 [details] wrong sizing of 'all workspace' windows 1) open to workspaces 2) open two windows on the first workspace 3) enable visible on all workspace for one of the two windows 4) go to overview mode result: the 'all workspace' window on the first workspace has wrong size. It looks like it has the same size as on the second workspace.
Created attachment 152286 [details] [review] work correct with 'all workspace' windows in overview
Comment on attachment 152286 [details] [review] work correct with 'all workspace' windows in overview This patch seems to do at least two things (fix sizing of all-workspace windows in the overview, and fix selection of all-workspace windows), and maybe a third (see below), and so ought to be split into separate patches. > let windowWorkspaceNum = window.get_workspace().index(); > >+ if (workspaceNum !== undefined) >+ windowWorkspaceNum = workspaceNum; we generally haven't used "!== undefined" as an idiom. Also there's no need for a duplicate assignment here. I'd say: let windowWorkspaceNum = workspaceNum ? workspaceNum : window.get_workspace().index(); >@@ -346,11 +346,15 @@ MosaicView.prototype = { > lostWorkspaces = this._workspaces.splice(newNumWorkspaces); > } > >- // The new last workspace may be removable >- let newLastWorkspace = this._workspaces[this._workspaces.length - 1]; >- > // Figure out the new layout > this._positionWorkspaces(); >+ >+ if (newNumWorkspaces > oldNumWorkspaces) { >+ for (let w = oldNumWorkspaces; w < newNumWorkspaces; w++) { >+ this._workspaces[w].positionWindows(0); >+ } >+ } >+ > let newScale = this._workspaces[0].scale; > let newGridWidth = Math.ceil(Math.sqrt(newNumWorkspaces)); > let newGridHeight = Math.ceil(newNumWorkspaces / newGridWidth); what does this do? (This is the part I thought might be a 3rd bugfix.)
Created attachment 152370 [details] [review] fix sizing of all-workspace windows in the overview > let windowWorkspaceNum = workspaceNum ? workspaceNum : window.get_workspace().index(); Will not work (workspaceNum can be 0). We can use (workspaceNum || workspaceNum === 0). But it more complex(and unclear), then !== undefined. > what does this do? (This is the part I thought might be a 3rd bugfix.) New workspaces can contain windows and we need position them.
Created attachment 152371 [details] [review] fix selection of all-workspace windows
Created attachment 154222 [details] [review] fix sizing of all-workspace windows in the overview merge with HEAD
Created attachment 154223 [details] [review] fix selection of all-workspace windows merge with HEAD
Comment on attachment 154222 [details] [review] fix sizing of all-workspace windows in the overview looks good except: >+ // New workspaces can containt windows. >+ for (let w = oldNumWorkspaces; w < newNumWorkspaces; w++) { >+ this._workspaces[w].positionWindows(0); >+ } > // Slide new workspaces in from offscreen > for (let w = oldNumWorkspaces; w < newNumWorkspaces; w++) > this._workspaces[w].slideIn(oldScale); typo ("containt"), and also, those two loops can be merged together also, this is in the code for the grid view. Shouldn't a "positionWindows()" call be needed in the linear view case too? (If it's not needed there for some reason, then go ahead and commit, with just the fix above.)