Review of "Persian Calendar" version 105

Details Page Preview

Shows Persian date in the top panel. It shows: 1. Persian calendar 2. It can show, today is a holiday or not! 3. Show notification onDayChanged! 4. Date converter between Persian, Gregorian and Lunar Hijri 5. Events: 5.1. Official solar events. 5.2. Official lunar events. 5.3. Official international events. 5.4. Traditional Persian events. 5.5. Persian personages. Please “rate” here and “star” the project on GitHub. Please open an issue on GitHub if you found something or have an idea!

Extension Homepage
https://github.com/omid/Persian-Calendar-for-Gnome-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.

All Versions

Previous Reviews on this Version

JustPerfection rejected
1. Don't use `imports` in 45 package (line 47 `utils/gettext.js`): ```js // legacy const ByteArray = imports.byteArray; let prepare = (out instanceof Uint8Array) ? ByteArray.toString(out) : out.toString(); // new let decoder = new TextDecoder('utf-8'); let prepare = decoder.decode(out); ``` 2. Anything you are storing in `PersianCalendarPreferences` (as `this.`) won't let the garbage collector do its job. You have two options: - Make them local to the current function. - Clean up on window close request: ```js window.connect('close-request', () => { // clean up here }); ``` 3. You can use `GLib.get_home_dir()` to get the home dir. No need to use `../`. 4. Missed semicolon (line 35-36 `extension.js`). 5. Also do this in disable: ```js this._settings = null; this._gettext = null; ``` 6. In case you need path, you can simply do `this.path`. No need to use `this.dir`: https://gjs.guide/extensions/upgrading/gnome-shell-45.html#extensionutils
JustPerfection posted a review
7. btw, you cannot import Gtk to GNOME Shell process: https://gjs.guide/extensions/review-guidelines/review-guidelines.html#gtk-and-gdk-imports That's the new rule we've added to the ego review guidelines. Use `Clutter.TextDirection` instead: https://gjs-docs.gnome.org/clutter13~13/clutter.textdirection
omid posted a review
Thanks a lot. I think fixed the first six. But about the 7th, I don't know how I can do that without Gtk. I use `Gtk.get_locale_direction()`, `Gtk.TextDirection` and `Gtk.Justification`. What are the equivalent non-gtk versions?
JustPerfection posted a review
# `Clutter.TextDirection` https://gjs-docs.gnome.org/clutter13~13-textdirection/ # `clutter.get_default_text_direction()` https://gjs-docs.gnome.org/clutter13~13/clutter.get_default_text_direction # `Gtk.Justification` No alternative but you can create your own: ```js const JUSTIFICATION = { LEFT: 0, RIGHT: 1, CENTER: 2, FILL: 3, }; ```
omid posted a review
Thanks a lot... I'll fix some styles and send another version soon (one tiny change in style needs a logout/login :/ I wished they merged that branch before GS45!)
JustPerfection posted a review
We will have better toolbox scripts soon, so you can test them easier. Also, you can test via nested shell: https://gjs.guide/extensions/development/debugging.html#running-a-nested-gnome-shell
omid posted a review
cool, thanks