Review of "Coverflow Alt-Tab" version 67

Details Page Preview

Replacement of Alt-Tab, iterates through windows in a cover-flow manner.

Extension Homepage
https://github.com/dsheeler/CoverflowAltTab

No comments.

Diff Against

Files

Note: Binary files aren't shown on the web site. To see all files, please download the extension zipfile.

All Versions

Previous Reviews on this Version

JustPerfection rejected
The instance is getting created in line 144 manager.js without calling `destroy()` on disable. and still can't tell the timeout is getting fully removed on the actual destroy (line 170 switcher.js). TBH, the `Switcher` class (switcher.js) is hard to review. For example: - `_stopDestroying()` (line 363) - monitoring **is destroying** (line 939) - Reason to destroy (line 938) - Checking timeout id for doing many things (line 949) You should add this at the end of `_onDestroy()` or `destroy()`: ```js if (this._initialDelayTimeoutId !== 0) { GLib.Source.remove(this._initialDelayTimeoutId); } ```
dsheeler posted a review
The destroy method does not instantaneously remove the switcher. The destroy function starts animating windows to their pre-switcher positions on the screen 982 and 1000. On completion, each of those animations calls this._onPreviewDestroyComplete() (991, 1006). this._onPreviewDestroyComplete() is meant to count how many animations are finished (each finished animation increments a counter (1085) and on 1086, if this is the last preview to animate into place, then the rest of the function executes which checks the timeouts 1092, 1097 and acts like the end of destroy(). Does that make sense?
JustPerfection posted a review
Well, that means the instance won't get destroyed on disable (against ego rules). Instead, it is relying on the animation to get destroyed.