GNOME Bugzilla – Bug 597044
More flexible interface for adding widgets to Chrome
Last modified: 2009-11-23 18:51:09 UTC
The current functions for adding widgets to Chrome as rather awkward, and doesn't allow the full range of possibilities: addActor: function(actor, shapeActor); Add an actor, optionally using shapeActor for its shape instead of the actor itself. setVisibleInOverview Modify a previously added actor to not be visible in the overview. addInputRegionActor Add a child of a previously added actor to the input region but not to the struts One possibility that is missing is adding a new actor and making it part of the input region but not the struts. I think it should be something like: addActor(actor, { visibleInOverview: true, shapeActor: someActor, affectsInputRegion: true, affectsStruts: false }); (With appropriate defaults.) The "child of a previously added actor" thing can be done by simply not reparenting actors that are already descendants of the chrome to the chrome - the check is done as an assertion right now.
Created attachment 147593 [details] [review] Add params.js, for function param object parsing. Use it in AppIcon._init
Created attachment 147594 [details] [review] [Chrome] clean up APIs and remove workarounds
Review of attachment 147593 [details] [review]: This looks good to me. There are various recent places where we are using bitfield flags for similar purposes. I like this approach better and think we should generally do this rather than the bitfield. ::: js/misc/params.js @@ +11,3 @@ +// contains any properties that aren't in @defaults. +// +// If @params is %null, this returns @defaults. This should say specificalyl that @params is both modified and returned; that seems like a reasonable behavior for efficiency but it's not something that someone would necessarily guess.
Review of attachment 147594 [details] [review]: Mostly looks like a nice cleanup; simplifies, removes code. But I don't really understand how untracking actors works here. ::: js/ui/chrome.js @@ +104,3 @@ + // Undoes the effect of trackActor() + untrackActor: function(actor) { + this._untrackActor(actor, true, true); Doesn't passing in true, true here mess up the reference counting? Similarly, doesn't the reference counting get messed up by removeActor?
Created attachment 147923 [details] [review] [Chrome] clean up APIs and remove workarounds The tracking code was trying to be too clever before and support wacky features that we didn't need anyway. This version gets rid of that, solving the "untrack messes up refcounting" problem (which wasn't actually a problem before, it just looked like it was because of the wacky bits that we weren't using).
Review of attachment 147923 [details] [review]: I no longer remember why we were recursively tracking. If it works, it should be fine.
committed. I removed the part of the params.js patch that ported appIcon to use it though, since that served little purpose at this point besides conflicting with Colins AppWell CSS port.