Review of "Auto Day/Night Theme Switch [DEPRECATED]" version 2

Details Page Preview

Automatically switch themes depending on the time of the day NOTE: Deprecated for GNOME > 3.36, please use Night Theme Switcher instead

Extension Homepage
https://github.com/Darazaki/AutoDayNightThemeSwitch

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
2 Active
1 Active

Previous Reviews on this Version

Darazaki posted a review
Hello! Here's a copy of the changelog I've published on GitHub: Added: - Ability for the preferences panel to be translated - German translation (thanks to Etamuk) - Compatibility with GNOME Shell 3.28 Fixed: - Shell themes now work on GNOME Shell 3.34.0 (previously only 3.34.1 or later) - Possible memory leak when extension got disabled after being enabled (thanks to andyholmes) Removed: - Broken shell themes support on GNOME Shell 3.32 or earlier (*) (*) The shell themes part of the preferences panel is now hidden on the incompatible versions
andyholmes active
Sorry, I should have explained a little better about signals :) You've got the right idea, however disconnecting signals is done by passing the number returned by `connect()` back to `disconnect()`. This allows you to have multiple connections to the same signal. ``` // You could store each on an object this._signalNameId = someObject.connect('signal-name', () => {}); someObject.disconnect(this._signalNameId); // Or maybe a list let handlerIds = []; handlerIds.push(someObject.connect('signal-name', () => {})); handlerIds.push(someObject.connect('signal2-name', () => {})); for (let id of handlerIds) someObject.disconnect(id); // Or do something fancier this._signalConnections = new Map([ [someObject, [1, 2, 3]], [someOtherObject, [1]], ]); for (let [obj, ids] of this._signalConnections.entries()) { for (let id of ids) obj.disconnect(id); } ``` I'll approve this any ways though, since those will just be warnings :)
Darazaki posted a review
Oh god I'm stupid, that's what I get for skimming through the documentation Thank you very much for pointing that out and your explaination, I'm working on a full rewrite so I'll fix it there Sorry since the diff's going to be pretty big, I'll try to split everything in several files