Review of "Time Tracker" version 4

Details Page Preview

Helps track time

Extension Homepage
https://github.com/jsnjack/time-tracker/

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
25 Active
24 Rejected
23 Active
22 Active
21 Rejected
20 Active
19 Active
18 Active
17 Active
16 Active
15 Active
14 Active
13 Active
12 Active
11 Active
10 Active
9 Active
8 Active
7 Active
6 Active
5 Active
4 Rejected
3 Active
2 Active
1 Rejected

Previous Reviews on this Version

fmuellner waiting for author
You probably want Date.toLocaleFormat() - the optional parameters to toLocaleString() have only been added in Firefox 29, so until gjs updates its JS engine to the corresponding version (I wouldn't count on it for 3.14), your change will not have any effect at all. Also the language codes don't seem to match the ones used by gettext[0], so it might not even work when gjs starts to support the new parameters. [0] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl#Locale_identification_and_negotiation
jsnjack posted a review
Yes. I wanted to use Date.toLocaleFormat(), but then I saw this big red banner on that page [1]. So I decided to use Date.toLocaleString() [2] because in examples locale names was the same as in GNOME Shell (now I noticed, that gnome uses underscore en_US and javascript uses en-US). So I prepared some code in Firefox console and copied it to my extension.js. I tested it with different regions and it worked fine. When you mentioned using toLocaleFormat(), I decided to check it in lg console and it generates error. After it I decided to check toLocaleString() in lg console. The thing is it does exactly what I need (I checked it by changing locale settings to different regions), but it completely ignores locale and options arguments! So I think I just need to replace start_time.toLocaleString(locale, options) with start_time.toLocaleString(). [1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toLocaleFormat [2] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toLocaleString
fmuellner posted a review
Well yes, the toLocaleString() options will be supported once gjs updates its JS engine (as mentioned before). If you read the toLocaleFormat() warning, it advises you to not use this non-standard feature on a page facing the public web - this obviously does not apply to gnome-shell/extensions. But sure, if you don't need more fine-grained control than date.toLocaleFormat('%c'), then date.toLocaleString() is perfectly fine.
jsnjack posted a review
Thanks! I will submit corrected version in a moment. By the way, could you please help me figure out how do settings works? For example, I have a key in my xml configuration file: <key name="show-seconds" type="b"> <default>false</default> <summary>Show seconds</summary> <description>Set to true to show seconds</description> </key> I would like to it to be shown in dconf editor as checkbox (helpful for debug), but it isn't shown at all! I looked through some others extensions which have the same checkbox I would like to have and I see no general difference in our xml files...
fmuellner posted a review
Yeah, dconf-editor only works for schemas that have been installed in the global schema dir. However after compiling your schema, something like gsettings --schemadir /path/to/extension/schema/dir your.extensions.schema.identifier show-seconds should work. But maybe it makes sense to follow the global clock-show-seconds in org.gnome.desktop.interface instead?
fmuellner rejected