Review of "TouchUp [beta]" version 2

Details Page Preview

An extension for Gnome Shell that enhances the user experience for devices with a touchscreen – primarily focused (and tested) on tablets. Current Features: - A system navigation bar (gesture or button mode) - Touch gestures for system notifications (dismiss, expand, …) - Make the system notification area touch-scrollable - Onscreen-Keyboard popups - A floating screen rotate button when auto-rotate is disabled Each of these features can be enabled/disabled individually and most can be further customized in the extension settings. A lot of additional features are planned – you can have a look at the repository for details on that. If you find bugs, have new ideas or would like to contribute, please open an issue on github so we can chat. Please be aware that this extension is currently in beta status; a few bugs are known and more are expected – if you find any issues, please create an issue on github to let me know.

Extension Homepage
https://github.com/mityax/gnome-extension-touchup

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

Version Status
1.1.0 (4) Active
3 Active
2 Rejected
1 Rejected

Previous Reviews on this Version

JustPerfection rejected
1. You should call `this._injectionManager.clear()` on disable (line 13 `utils/patchManager.js`). 2. Destroy and Null out everything in disable (line 16-24 `extension.js`): [EGO Review Guidelines: Destroy](https://gjs.guide/extensions/review-guidelines/review-guidelines.html#destroy-all-objects)
mityax posted a review
1. ✅ Good catch, added 2. That's already being handled by `PatchManager.destroy()`, as the `defineFeature` method, upon `PatchManager` destruction, calls it's `assign()` callback argument with `undefined` as it's value (line 87, extension.js). It's also document in the method's docsting and the comment in line 87, but I made it a bit more explicit in the new version to ease review in the future. Is it okay this way?
JustPerfection posted a review
Each class should be responsible for its own property. Avoid using callbacks for cleaning up like that. On disable or destroy of each class, you should explicitly clean up. The code you send here is meant to be reviewed. Please make it more readable.