GNOME Bugzilla – Bug 596000
Add lightbox.js and use it in overview, alt-tab, and run dialog
Last modified: 2009-09-22 21:24:53 UTC
As mentioned in passing in other bugs, here's an abstraction for highlighting particular actors in a group by using a dark translucent actor to obscure the others
Created attachment 143746 [details] [review] [lightbox] Add lightbox.js and use it in overview, alt-tab, and run dialog
Review of attachment 143746 [details] [review]: Like it! only minor style comments. ::: /dev/null @@ +85,3 @@ + // Somewhere else; insert it into the correct spot + let nextChild = this._children.indexOf(children[newChildIndex + 1]); + if (nextChild != -1) Might be good to add a // paranoia comment to confirm the reader's guess that this shouldn't happen. @@ +92,3 @@ + _actorRemoved : function(container, child) { + let index = this._children.indexOf(child); + if (index != -1) Also // paranoia, right? @@ +121,3 @@ + } else if (this._children[i] == this._highlighted) { + this._children[i].lower(below); + below = this._children[i]; You don't need to set below here, right? @@ +130,3 @@ + + destroy : function() { + if (this._allocationChangedSignalId) I don't like testing undefined property members for truth - I have a dream that somebody we can turn on a JS warning that will catch errors. I'd say either set it to null explicitly or test '=== undefined' here. (Or maybe test '== null' here but I don't really like testing undefined members against null either for similar reasons.)
> You don't need to set below here, right? right. leftover from an earlier state where I allowed highlighting multiple actors at once, because I'd initially thought the overview did that sometimes. Attachment 143746 [details] pushed as 3734479 - [lightbox] Add lightbox.js and use it in overview, alt-tab, and run dialog