GNOME Bugzilla – Bug 642196
Split the overview's constructor into an internal and a more public part
Last modified: 2011-02-16 19:00:05 UTC
We currently jump through some hoops to work around the limitation of the overview not having finished constructing when initializing components like the dash and the view selector. Solve this by splitting out a public init() method, which is called in main.js after constructing the Overview. See comment 18 in bug 639428 for reference.
Created attachment 180743 [details] [review] dash: Remove show/hide functions As Main.overview is now usable from the dash's constructor, move the setup of signal connections there and remove the show/hide methods which were used as workaround.
Created attachment 180744 [details] [review] search-entry: Handle find-as-you-type activation internally To enable find-as-you-type when entering the overview and disabling it when leaving, we used a chain of functions calls from ViewSelector over SearchTab to SearchEntry. As find-as-you-type should be enabled while the overview is shown, the activation/deactivation can be handled entirely by the SearchEntry itself by tying it to the entry's visibility.
Created attachment 180745 [details] [review] view-selector: Remove show/hide functions As Main.overview is now usable from the view selector's constructor, move the setup of signal connections there and remove the show/hide methods which were used as workaround.
Created attachment 180746 [details] [review] overview: Split a public init() function out of the constructor The Overview does not only hold the different elements visible in the overview, but is also a central point to manage drag signals. As objects which are constructed in the overview constructor cannot access Main.overview (as its constructor has not finished yet), we use misnamed show/hide methods to work around this limitation, which are called when entering/leaving the overview. A better way to handle this problem is to remove the limitation altogether by splitting the overview constructor between internals, which remain in the constructor, and more complex objects which need to access Main.overview, and whose initialization is moved to a public init() function which is called by main.js after the overview has been constructed. Ooops, forgot one patch - this one should come first.
Review of attachment 180746 [details] [review]: ::: js/ui/overview.js @@ +170,3 @@ + }, + + init: function() { Might be good to have a comment explaining the basic concept between the split between what is done in the constructor and what is in init(). Or of there's no real concept - if everything is done in the constructor other than the stuff that has to be done after Main.overview exists - say that.
Review of attachment 180743 [details] [review]: Looks good
Review of attachment 180744 [details] [review]: Looks good - if clutter is going through all the work to propagate mapped notifications through the entire tree, we might as well use them!
Review of attachment 180745 [details] [review]: good!
(In reply to comment #5) > Might be good to have a comment explaining the basic concept between the split > between what is done in the constructor and what is in init(). Or of there's no > real concept - if everything is done in the constructor other than the stuff > that has to be done after Main.overview exists - say that. How about: + // Initialization of objects from JS prototypes has been split out + // of the constructor, so that Main.overview is available when + // their constructor is run. Accidentally, the split is also between private and public properties, should that be mentioned as well (though it's really accidental)?
Suggest: // The members we construct that are implemented in JS might want to // access the overview as Main.overview to connect signal handlers // and so forth. So we create them after construction in this init() // method. Or something like that.
Attachment 180743 [details] pushed as f1c2797 - dash: Remove show/hide functions Attachment 180744 [details] pushed as 0ae44f4 - search-entry: Handle find-as-you-type activation internally Attachment 180745 [details] pushed as 6b429b7 - view-selector: Remove show/hide functions Attachment 180746 [details] pushed as f39e693 - overview: Split a public init() function out of the constructor