Review of "Nowa Shell" version 2

Details Page Preview

Light visual and usability adjustments for GNOME Shell. - Fine-tuned shell appearance with corrected border radius and padding; - Subtly rounded screen corners; - Dedicated notification panel separated from calendar; - Calendar with enhanced event list; - Suppresses redundant notifications ("app is ready", "app has been pinned"); - Power panel with direct actions, without confirmation dialogs; - Quick Settings Dark Style toggle with automatic switching based on sunset and sunrise times; - Optional minimalist Dash to Dock theme;

Extension Homepage
https://github.com/nowaos/ext-nowa-shell

No comments.

Diff Against

Files

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

Shexli (experimental)

Shexli has not produced a result yet.

All Versions

Version Status
3 Active
2 Rejected
1 Rejected

Previous Reviews on this Version

alxmagro posted a review
- Minor bug fix;
JustPerfection rejected
1. Please remove `schemas/gschemas.compiled`. Not needed for 45+ packages. 2. You have logger. No need for `src/services/Logger.js`: [Port Guide 48: Custom Logger](https://gjs.guide/extensions/upgrading/gnome-shell-48.html#custom-logger) 3. Timeout should be removed on destroy: - line 48 `src/modules/CalendarManager.js` - line 106 `src/views/CustomCalendar/index.js` [EGO Review Guidelines: Timeout](https://gjs.guide/extensions/review-guidelines/review-guidelines.html#remove-main-loop-sources) 4. Using `this.gettext()` from the entry is prefered instead of `src/interfaces/translations.js`. [Extensions Guide: Gettext](https://gjs.guide/extensions/topics/extension.html#gettext) By using that, only specifying `gettext-domain` in `metadata.json` would be enough. 5. Better using `this.path` instead of `this.dir.get_path()`. 6. Avoid using unnecessary try-catch when something doesn't trhow. For example, line 60 `src/views/NotificationButton/index.js`. 7. This is not really needed: ```js destroy () { super.destroy() } ``` 8. What's the reason for using `onDestroy()`? Why not simply using `destroy()`? 9. Using `_` for `_BaseView` means that is a private class but you are exporting that. I recommend to name it `BaseView` instead. Btw, thanks for keeping the code clean while sending a large code. and sorry for the review delay. For future updates, avoid sending large diff. so it gets reviewed faster. You can also add `donations` to the `metadata.json`, so people can donate to you if they want: [Extension Anatomy: donations](https://gjs.guide/extensions/overview/anatomy.html#donations) If you need any help with your extension you can ask us on: - [GNOME Extensions Matrix Channel](https://matrix.to/#/#extensions:gnome.org) - IRC Bridge: irc://irc.gimpnet.org/shell-extensions
alxmagro posted a review
1. Fixed; 2. Addressed below*; 3. Fixed; 4. Addressed below*; 5. `this.path` does not exist inside the DashToDockThemer submodule. I’m passing `dir` from extension.js to all submodules through BaseModule; 6. Fixed; 7. In BaseView, I decided to create a "component" with a minimal lifecycle skeleton. In future releases, I can implement automatic cleanup of timeouts or other resources directly in `destroy`, without requiring child classes to worry about calling this in `onDestroy`. * Regarding points 2 and 4, I appreciated learning about those helpers. However, if the change is not mandatory, I prefer to keep the current structure, since the codebase follows a decentralized structure. It would become more difficult if I had to load helpers from extension.js into the components, for example: ``` extension.js -> CalendarManager -> DateMenuService -> CustomCalendar (log here) ```