Created attachment 163623 [details] [review]
[dash, src] remove ShellOverflowList and related unused code

The dash had code that created a ShellOverflowList in some situations,
but those situations never happen in the current code. So remove that
dead code.

Nothing else was using ShellOverflowList ever, so remove it. (Even if
we wanted that sort of functionality again later, we'd want it to be
StWidget-based.)

(In reply to comment #0)
> In the process, I noticed that GenericDisplay is actually only used by
> DocDisplay now, and probably ought to be merged into it... unless anyone
> knows of any upcoming UI changes that will make it worthwhile to keep that
> distinction?

Not really, but the mockups referenced in bug 617747 look like it's probably easier to either start from scratch or factor out a new base class from appDisplay.

Review of attachment 163623 [details] [review]:

Looks good - if you ignore the suggestion below you can commit with the typo fixed.

::: js/ui/dash.js
@@ +44,2 @@
     if (displayType == DOCS)
+        return new DocDisplay.DocDisplay();

Quickly grepping through the code base suggests that the whole displayType mechanism is now nothing but an obfuscated call to new DocDisplay.DocDisplay():

_createDisplay(displayType) is called from
new ResultArea(displayType) is called from
ResultPane.packResults(DOCS)

I wouldn't target the whole mess right now, but removing the displayType parameter and the _createDisplay function looks like it fits in with this patch.

::: js/ui/docDisplay.js
@@ +119,1 @@
     this._init(flags);

Capture the flag!

Created attachment 163636 [details] [review]
[dash, src] remove ShellOverflowList and related unused code

remove the additional "flags" you noticed

Created attachment 163637 [details] [review]
remove _createDisplay

(to be squashed)

Created attachment 163638 [details] [review]
and remove some more unnecessary abstraction

(to be squashed)

Created attachment 163639 [details] [review]
[dash] remove some old code that's no longer used

from my keynav branch, since this is turning into a general dash cleanup
bug...

Review of attachment 163636 [details] [review]:

Looks good.

Review of attachment 163636 [details] [review]:

Looks good.

Review of attachment 163637 [details] [review]:

Looks good.

Review of attachment 163638 [details] [review]:

Looks good, but now that we're at it - there's room for more!

::: js/ui/dash.js
@@ +136,3 @@
     _init: function(dash) {
         Pane.prototype._init.call(this);
 

Those nested anonymous functions handle a signal which is never triggered (it is emitted as GenericDisplay::show-details from a signal handler connected to GenericDisplayItem::show-details - neither GenericDisplayItem nor DocDisplayItem emit this signal though).

So the entire prototype can actually be reduced to this:

ResultPane.prototype = {
    __proto__: Pane.prototype,

    _init: function(dash) {
        Pane.prototype._init.call(this);

        let resultArea = new ResultArea();
        this.actor.add(resultArea.actor, { expand: true });
        this.connect('open-state-changed',
            Lang.bind(this, function(pane, isOpen) {
                resultArea.display.resetState();
        }));
    }
};

Review of attachment 163639 [details] [review]:

Oh my - I think after all it _is_ a good idea to do a similar clean-up run on docDisplay/genericDisplay ...

(In reply to comment #12)
> Oh my - I think after all it _is_ a good idea to do a similar clean-up run on
> docDisplay/genericDisplay ...

I'd thought that, but probably the new design is just going to throw out the old code mostly, so...

I guess at least, whoever does it should keep this in mind.

made the additional fix you suggested, then rebased a whole bunch and
committed