GNOME Bugzilla – Bug 655813
layout/chrome reorg/cleanup for on-screen keyboard
Last modified: 2011-08-03 13:21:06 UTC
The way the on-screen keyboard and message tray are supposed to work together is: when the keyboard is hidden, the tray works like it always has. But when the keyboard is shown, the keyboard is at the bottom of the screen, and the tray is above the keyboard (and hides itself behind the keyboard when it's not needed). Since you don't want either the tray or the keyboard to have to know too much about the other's state/logic, we moved the positioning code for them both into LayoutManager. This required some reorganization and cleanup....
Created attachment 193067 [details] [review] chrome: Make affectsStruts default to false Since we only want it to be true for the panel, and nothing else.
Created attachment 193068 [details] [review] panel: move the corners into the panel actor Rather than having the panel corners as independent bits of chrome and manually syncing their positions, put them inside the panel actor, and update the panel's allocation code to position them correctly.
Created attachment 193069 [details] [review] messageTray: move the summary notification out of MessageTray.actor With the old pre-boxpointer summary notifications, it sort of made sense that the summary notification actor was a child of the message tray. But there's no reason for that now, and in fact, it ends up requiring special cases in some places since hovering over the summary notification counts as hovering over the tray. So, fix this.
Created attachment 193070 [details] [review] lookingGlass: put this in the chrome layer Looking Glass is supposed to slide out from underneath the panel. Rather than fiddling with Main.chrome.actor directly, just add the lg actor to the chrome, and fix its stacking there.
Created attachment 193071 [details] [review] layout: merge chrome.js into layout.js LayoutManager and Chrome are already somewhat intertwined and will be becoming more so. As a first step in merging them, move the Chrome object into layout.js (with no other code changes).
Created attachment 193072 [details] [review] layout: make Chrome an implementation detail of LayoutManager Make the Chrome object be a private member of LayoutManager, and add new LayoutManager methods to proxy to the old Chrome methods.
Created attachment 193073 [details] [review] layout: add panelBox and trayBox Have LayoutManager automatically deal with sizing and positioning boxes for the panel and messageTray relative to the monitors. Also, now that LayoutManager knows exactly where and how tall the panel and tray are, have it manage the pointer barriers as well.
Comment on attachment 193073 [details] [review] layout: add panelBox and trayBox this patch is not in its final form yet, but I'm including it to show what the other patches are leading to
mclasen told me I should assign landing-the-keyboard bugs to florian :)
Review of attachment 193067 [details] [review]: LGTM
Review of attachment 193068 [details] [review]: LGTM
Review of attachment 193069 [details] [review]: Sure.
Review of attachment 193070 [details] [review]: Sure. ::: js/ui/lookingGlass.js @@ +996,3 @@ Main.popModal(this._entry); + this.actor.lower_bottom(); Is this necessary?
Review of attachment 193071 [details] [review]: Sure.
Review of attachment 193072 [details] [review]: I assume I miss some context from "future" patches and therefore fail to see the usefulness of this change; in particular the circular references of LayoutManager._chrome/Chrome._layoutManager are a bit ugly - maybe it makes sense to drop the Chrome object altogether and merge the code into LayoutManager?
(In reply to comment #13) > + this.actor.lower_bottom(); > > Is this necessary? Yes. Otherwise in at least some circumstances the lg window goes over the panel instead of under it when it's sliding out. (In reply to comment #15) > Review of attachment 193072 [details] [review]: > > I assume I miss some context from "future" patches and therefore fail to see > the usefulness of this change; in particular the circular references of > LayoutManager._chrome/Chrome._layoutManager are a bit ugly The circular reference was already there, it's just that they used to refer to each other via Main.chrome and Main.layoutManager, which therefore required a little bit of jumping-through-hoops at initialization time, which got worse when LayoutManager started getting more involved with specific objects (eg, we needed to create the LayoutManager, then create the Chrome, then call another LayoutManager init function to put the panelBox, trayBox, etc into Main.chrome, then create the Panel, MessageTray, etc, which need to refer to Main.layoutManager.panelBox, etc, then later call ANOTHER LayoutManager function to do the updateHotCorners call that needs access to Main.panel, etc.) > maybe it makes > sense to drop the Chrome object altogether and merge the code into > LayoutManager? I tried that, but the formerly-chrome and formerly-layoutmanager bits remained mostly separate from one another even after "merging". But yes, now that Chrome is private to the LayoutManager, we could do that in the future. I figured I'd look at it again after rebasing bug 633620 on top of this.
Attachment 193067 [details] pushed as a376cd1 - chrome: Make affectsStruts default to false Attachment 193068 [details] pushed as fde200d - panel: move the corners into the panel actor Attachment 193069 [details] pushed as 446910c - messageTray: move the summary notification out of MessageTray.actor Attachment 193070 [details] pushed as bcd3070 - lookingGlass: put this in the chrome layer Attachment 193071 [details] pushed as 99149f9 - layout: merge chrome.js into layout.js Attachment 193072 [details] pushed as c1acf99 - layout: make Chrome an implementation detail of LayoutManager